Use f_lock to protect f_flags
Jonathan Corbet [Fri, 6 Feb 2009 22:25:24 +0000 (15:25 -0700)]
Traditionally, changes to struct file->f_flags have been done under BKL
protection, or with no protection at all.  This patch causes all f_flags
changes after file open/creation time to be done under protection of
f_lock.  This allows the removal of some BKL usage and fixes a number of
longstanding (if microscopic) races.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Jonathan Corbet <corbet@lwn.net>

13 files changed:
drivers/char/tty_io.c
drivers/usb/gadget/file_storage.c
fs/fcntl.c
fs/ioctl.c
fs/nfsd/vfs.c
include/linux/fs.h
ipc/mqueue.c
sound/core/oss/pcm_oss.c
sound/oss/au1550_ac97.c
sound/oss/audio.c
sound/oss/sh_dac_audio.c
sound/oss/swarm_cs4297a.c
sound/oss/vwsnd.c

index bc84e12..224f271 100644 (file)
@@ -2162,13 +2162,12 @@ static int fionbio(struct file *file, int __user *p)
        if (get_user(nonblock, p))
                return -EFAULT;
 
-       /* file->f_flags is still BKL protected in the fs layer - vomit */
-       lock_kernel();
+       spin_lock(&file->f_lock);
        if (nonblock)
                file->f_flags |= O_NONBLOCK;
        else
                file->f_flags &= ~O_NONBLOCK;
-       unlock_kernel();
+       spin_unlock(&file->f_lock);
        return 0;
 }
 
index 1ab9dac..33bb76c 100644 (file)
@@ -1711,7 +1711,9 @@ static int do_write(struct fsg_dev *fsg)
                curlun->sense_data = SS_WRITE_PROTECTED;
                return -EINVAL;
        }
+       spin_lock(&curlun->filp->f_lock);
        curlun->filp->f_flags &= ~O_SYNC;       // Default is not to wait
+       spin_unlock(&curlun->filp->f_lock);
 
        /* Get the starting Logical Block Address and check that it's
         * not too big */
@@ -1728,8 +1730,11 @@ static int do_write(struct fsg_dev *fsg)
                        curlun->sense_data = SS_INVALID_FIELD_IN_CDB;
                        return -EINVAL;
                }
-               if (fsg->cmnd[1] & 0x08)        // FUA
+               if (fsg->cmnd[1] & 0x08) {      // FUA
+                       spin_lock(&curlun->filp->f_lock);
                        curlun->filp->f_flags |= O_SYNC;
+                       spin_unlock(&curlun->filp->f_lock);
+               }
        }
        if (lba >= curlun->num_sectors) {
                curlun->sense_data = SS_LOGICAL_BLOCK_ADDRESS_OUT_OF_RANGE;
index bd215cc..04df857 100644 (file)
@@ -189,7 +189,9 @@ static int setfl(int fd, struct file * filp, unsigned long arg)
                }
        }
 
+       spin_lock(&filp->f_lock);
        filp->f_flags = (arg & SETFL_MASK) | (filp->f_flags & ~SETFL_MASK);
+       spin_unlock(&filp->f_lock);
  out:
        unlock_kernel();
        return error;
index 240ec63..421aab4 100644 (file)
@@ -404,10 +404,12 @@ static int ioctl_fionbio(struct file *filp, int __user *argp)
        if (O_NONBLOCK != O_NDELAY)
                flag |= O_NDELAY;
 #endif
+       spin_lock(&filp->f_lock);
        if (on)
                filp->f_flags |= flag;
        else
                filp->f_flags &= ~flag;
+       spin_unlock(&filp->f_lock);
        return error;
 }
 
@@ -432,10 +434,12 @@ static int ioctl_fioasync(unsigned int fd, struct file *filp,
        if (error)
                return error;
 
+       spin_lock(&filp->f_lock);
        if (on)
                filp->f_flags |= FASYNC;
        else
                filp->f_flags &= ~FASYNC;
+       spin_unlock(&filp->f_lock);
        return error;
 }
 
@@ -499,10 +503,7 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
                break;
 
        case FIONBIO:
-               /* BKL needed to avoid races tweaking f_flags */
-               lock_kernel();
                error = ioctl_fionbio(filp, argp);
-               unlock_kernel();
                break;
 
        case FIOASYNC:
index 6e50aaa..c165a64 100644 (file)
@@ -998,8 +998,11 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
 
        if (!EX_ISSYNC(exp))
                stable = 0;
-       if (stable && !EX_WGATHER(exp))
+       if (stable && !EX_WGATHER(exp)) {
+               spin_lock(&file->f_lock);
                file->f_flags |= O_SYNC;
+               spin_unlock(&file->f_lock);
+       }
 
        /* Write the data. */
        oldfs = get_fs(); set_fs(KERNEL_DS);
index 2011600..7428c6d 100644 (file)
@@ -848,7 +848,7 @@ struct file {
 #define f_dentry       f_path.dentry
 #define f_vfsmnt       f_path.mnt
        const struct file_operations    *f_op;
-       spinlock_t              f_lock;  /* f_ep_links */
+       spinlock_t              f_lock;  /* f_ep_links, f_flags */
        atomic_long_t           f_count;
        unsigned int            f_flags;
        fmode_t                 f_mode;
index 54b4077..a8ddadb 100644 (file)
@@ -1156,10 +1156,12 @@ SYSCALL_DEFINE3(mq_getsetattr, mqd_t, mqdes,
        omqstat.mq_flags = filp->f_flags & O_NONBLOCK;
        if (u_mqstat) {
                audit_mq_getsetattr(mqdes, &mqstat);
+               spin_lock(&filp->f_lock);
                if (mqstat.mq_flags & O_NONBLOCK)
                        filp->f_flags |= O_NONBLOCK;
                else
                        filp->f_flags &= ~O_NONBLOCK;
+               spin_unlock(&filp->f_lock);
 
                inode->i_atime = inode->i_ctime = CURRENT_TIME;
        }
index 0a1798e..d4460f1 100644 (file)
@@ -1895,7 +1895,9 @@ static int snd_pcm_oss_set_fragment(struct snd_pcm_oss_file *pcm_oss_file, unsig
 
 static int snd_pcm_oss_nonblock(struct file * file)
 {
+       spin_lock(&file->f_lock);
        file->f_flags |= O_NONBLOCK;
+       spin_unlock(&file->f_lock);
        return 0;
 }
 
index 81e1f44..4191acc 100644 (file)
@@ -1627,7 +1627,9 @@ au1550_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
                                    sizeof(abinfo)) ? -EFAULT : 0;
 
        case SNDCTL_DSP_NONBLOCK:
+               spin_lock(&file->f_lock);
                file->f_flags |= O_NONBLOCK;
+               spin_unlock(&file->f_lock);
                return 0;
 
        case SNDCTL_DSP_GETODELAY:
index 89bd27a..b69c05b 100644 (file)
@@ -433,7 +433,9 @@ int audio_ioctl(int dev, struct file *file, unsigned int cmd, void __user *arg)
                        return dma_ioctl(dev, cmd, arg);
                
                case SNDCTL_DSP_NONBLOCK:
+                       spin_lock(&file->f_lock);
                        file->f_flags |= O_NONBLOCK;
+                       spin_unlock(&file->f_lock);
                        return 0;
 
                case SNDCTL_DSP_GETCAPS:
index e5d4239..78cfb66 100644 (file)
@@ -135,7 +135,9 @@ static int dac_audio_ioctl(struct inode *inode, struct file *file,
                return put_user(AFMT_U8, (int *)arg);
 
        case SNDCTL_DSP_NONBLOCK:
+               spin_lock(&file->f_lock);
                file->f_flags |= O_NONBLOCK;
+               spin_unlock(&file->f_lock);
                return 0;
 
        case SNDCTL_DSP_GETCAPS:
index 41562ec..1edab7b 100644 (file)
@@ -2200,7 +2200,9 @@ static int cs4297a_ioctl(struct inode *inode, struct file *file,
                                    sizeof(abinfo)) ? -EFAULT : 0;
 
        case SNDCTL_DSP_NONBLOCK:
+               spin_lock(&file->f_lock);
                file->f_flags |= O_NONBLOCK;
+               spin_unlock(&file->f_lock);
                return 0;
 
        case SNDCTL_DSP_GETODELAY:
index 78b8acc..187f727 100644 (file)
@@ -2673,7 +2673,9 @@ static int vwsnd_audio_do_ioctl(struct inode *inode,
 
        case SNDCTL_DSP_NONBLOCK:       /* _SIO  ('P',14) */
                DBGX("SNDCTL_DSP_NONBLOCK\n");
+               spin_lock(&file->f_lock);
                file->f_flags |= O_NONBLOCK;
+               spin_unlock(&file->f_lock);
                return 0;
 
        case SNDCTL_DSP_RESET:          /* _SIO  ('P', 0) */