Remove BKL from remote_llseek v2

- Replace remote_llseek with generic_file_llseek_unlocked (to force compilation
failures in all users)
- Change all users to either use generic_file_llseek_unlocked directly or
take the BKL around. I changed the file systems who don't use the BKL
for anything (CIFS, GFS) to call it directly. NCPFS and SMBFS and NFS
take the BKL, but explicitely in their own source now.

I moved them all over in a single patch to avoid unbisectable sections.

Open problem: 32bit kernels can corrupt fpos because its modification
is not atomic, but they can do that anyways because there's other paths who
modify it without BKL.

Do we need a special lock for the pos/f_version = 0 checks?

Trond says the NFS BKL is likely not needed, but keep it for now
until his full audit.

v2: Use generic_file_llseek_unlocked instead of remote_llseek_unlocked
    and factor duplicated code (suggested by hch)

Cc: Trond.Myklebust@netapp.com
Cc: swhiteho@redhat.com
Cc: sfrench@samba.org
Cc: vandrove@vc.cvut.cz
Signed-off-by: Andi Kleen <ak@suse.de>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Jonathan Corbet <corbet@lwn.net>

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 427a7c695896ff87c1d463d595e29126d5aa5a44..aeff0fe5b6b9131c62743de404f4683b28ba6f00 100644 (file)
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -581,7 +581,7 @@ static loff_t cifs_llseek(struct file *file, loff_t offset, int origin)
                if (retval < 0)
                        return (loff_t)retval;
        }
-       return remote_llseek(file, offset, origin);
+       return generic_file_llseek_unlocked(file, offset, origin);
 }
 
 struct file_system_type cifs_fs_type = {

diff --git a/fs/gfs2/ops_file.c b/fs/gfs2/ops_file.c
index e1b7d525a06613f83abb239478cd0d8b8e09bb18..24dd59450088bd9ca949dd9c9e8c4770bb6223fc 100644 (file)
--- a/fs/gfs2/ops_file.c
+++ b/fs/gfs2/ops_file.c
@@ -62,11 +62,11 @@ static loff_t gfs2_llseek(struct file *file, loff_t offset, int origin)
                error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, LM_FLAG_ANY,
                                           &i_gh);
                if (!error) {
-                       error = remote_llseek(file, offset, origin);
+                       error = generic_file_llseek_unlocked(file, offset, origin);
                        gfs2_glock_dq_uninit(&i_gh);
                }
        } else
-               error = remote_llseek(file, offset, origin);
+               error = generic_file_llseek_unlocked(file, offset, origin);
 
        return error;
 }

diff --git a/fs/ncpfs/file.c b/fs/ncpfs/file.c
index 2b145de45b39147ce8e4ab9a53fd709c614050de..6a7d901f1936e555a530981c6fc6e53bab000f00 100644 (file)
--- a/fs/ncpfs/file.c
+++ b/fs/ncpfs/file.c
@@ -18,6 +18,7 @@
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/sched.h>
+#include <linux/smp_lock.h>
 
 #include <linux/ncp_fs.h>
 #include "ncplib_kernel.h"
@@ -281,9 +282,18 @@ static int ncp_release(struct inode *inode, struct file *file) {
        return 0;
 }
 
+static loff_t ncp_remote_llseek(struct file *file, loff_t offset, int origin)
+{
+       loff_t ret;
+       lock_kernel();
+       ret = generic_file_llseek_unlocked(file, offset, origin);
+       unlock_kernel();
+       return ret;
+}
+
 const struct file_operations ncp_file_operations =
 {
-       .llseek         = remote_llseek,
+       .llseek         = ncp_remote_llseek,
        .read           = ncp_file_read,
        .write          = ncp_file_write,
        .ioctl          = ncp_ioctl,

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 3536b01164f9ca036aa102ee5211d670273f26df..a34eb78989f5bfce1dafe12cb69c8698b1ca2483 100644 (file)
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -170,6 +170,7 @@ force_reval:
 
 static loff_t nfs_file_llseek(struct file *filp, loff_t offset, int origin)
 {
+       loff_t loff;
        /* origin == SEEK_END => we must revalidate the cached file length */
        if (origin == SEEK_END) {
                struct inode *inode = filp->f_mapping->host;
@@ -177,7 +178,10 @@ static loff_t nfs_file_llseek(struct file *filp, loff_t offset, int origin)
                if (retval < 0)
                        return (loff_t)retval;
        }
-       return remote_llseek(filp, offset, origin);
+       lock_kernel();  /* BKL needed? */
+       loff = generic_file_llseek_unlocked(filp, offset, origin);
+       unlock_kernel();
+       return loff;
 }
 
 /*

diff --git a/fs/read_write.c b/fs/read_write.c
index f0d1240a5c6990c7639948fd9e2ce56f613cda2f..9ba495d5a29b26d9493eea556ccd0992deb68829 100644 (file)
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -31,12 +31,12 @@ const struct file_operations generic_ro_fops = {
 
 EXPORT_SYMBOL(generic_ro_fops);
 
-loff_t generic_file_llseek(struct file *file, loff_t offset, int origin)
+loff_t
+generic_file_llseek_unlocked(struct file *file, loff_t offset, int origin)
 {
        loff_t retval;
        struct inode *inode = file->f_mapping->host;
 
-       mutex_lock(&inode->i_mutex);
        switch (origin) {
                case SEEK_END:
                        offset += inode->i_size;
@@ -46,42 +46,26 @@ loff_t generic_file_llseek(struct file *file, loff_t offset, int origin)
        }
        retval = -EINVAL;
        if (offset>=0 && offset<=inode->i_sb->s_maxbytes) {
+               /* Special lock needed here? */
                if (offset != file->f_pos) {
                        file->f_pos = offset;
                        file->f_version = 0;
                }
                retval = offset;
        }
-       mutex_unlock(&inode->i_mutex);
        return retval;
 }
+EXPORT_SYMBOL(generic_file_llseek_unlocked);
 
-EXPORT_SYMBOL(generic_file_llseek);
-
-loff_t remote_llseek(struct file *file, loff_t offset, int origin)
+loff_t generic_file_llseek(struct file *file, loff_t offset, int origin)
 {
-       loff_t retval;
-
-       lock_kernel();
-       switch (origin) {
-               case SEEK_END:
-                       offset += i_size_read(file->f_path.dentry->d_inode);
-                       break;
-               case SEEK_CUR:
-                       offset += file->f_pos;
-       }
-       retval = -EINVAL;
-       if (offset>=0 && offset<=file->f_path.dentry->d_inode->i_sb->s_maxbytes) {
-               if (offset != file->f_pos) {
-                       file->f_pos = offset;
-                       file->f_version = 0;
-               }
-               retval = offset;
-       }
-       unlock_kernel();
-       return retval;
+       loff_t n;
+       mutex_lock(&file->f_dentry->d_inode->i_mutex);
+       n = generic_file_llseek_unlocked(file, offset, origin);
+       mutex_unlock(&file->f_dentry->d_inode->i_mutex);
+       return n;
 }
-EXPORT_SYMBOL(remote_llseek);
+EXPORT_SYMBOL(generic_file_llseek);
 
 loff_t no_llseek(struct file *file, loff_t offset, int origin)
 {

diff --git a/fs/smbfs/file.c b/fs/smbfs/file.c
index efbe29af3d7a030bbd531786edbeb15a107eaf48..2294783320cbd14a1b88cd2822122bed664e42cf 100644 (file)
--- a/fs/smbfs/file.c
+++ b/fs/smbfs/file.c
@@ -422,9 +422,18 @@ smb_file_permission(struct inode *inode, int mask, struct nameidata *nd)
        return error;
 }
 
+static loff_t smb_remote_llseek(struct file *file, loff_t offset, int origin)
+{
+       loff_t ret;
+       lock_kernel();
+       ret = generic_file_llseek_unlocked(file, offset, origin);
+       unlock_kernel();
+       return ret;
+}
+
 const struct file_operations smb_file_operations =
 {
-       .llseek         = remote_llseek,
+       .llseek         = smb_remote_llseek,
        .read           = do_sync_read,
        .aio_read       = smb_file_aio_read,
        .write          = do_sync_write,

diff --git a/include/linux/fs.h b/include/linux/fs.h
index f413085f748e574140ca4216abbb303209cc7fb0..b158e5161bcaffdc20107fc3de3413c9fd09363e 100644 (file)
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1871,7 +1871,8 @@ extern void
 file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping);
 extern loff_t no_llseek(struct file *file, loff_t offset, int origin);
 extern loff_t generic_file_llseek(struct file *file, loff_t offset, int origin);
-extern loff_t remote_llseek(struct file *file, loff_t offset, int origin);
+extern loff_t generic_file_llseek_unlocked(struct file *file, loff_t offset,
+                       int origin);
 extern int generic_file_open(struct inode * inode, struct file * filp);
 extern int nonseekable_open(struct inode * inode, struct file * filp);