[PATCH] ->setattr() locking changes

Take ->i_sem in all callers of notify_change().

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 08a52c4f97643c9907b5ed17eed2e409fd558667..66f8d167d67a3d7ca32d1fbf949e7316c45d66fb 100644 (file)
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -65,7 +65,7 @@ rename:               no      yes (all)       (see below)
 readlink:      no      no
 follow_link:   no      no
 truncate:      no      yes             (see below)
-setattr:       yes     if ATTR_SIZE
+setattr:       no      yes
 permission:    yes     no
 getattr:                               (see below)
 revalidate:    no                      (see below)

diff --git a/Documentation/filesystems/porting b/Documentation/filesystems/porting
index af16ee3c949e5d236565ce3f73c8a65449a2fc2a..1ac5d3af530e86979018efeb9a69670f150b164e 100644 (file)
--- a/Documentation/filesystems/porting
+++ b/Documentation/filesystems/porting
@@ -116,3 +116,10 @@ FS_LITTER is gone - just remove it from fs_flags.
        FS_SINGLE is gone (actually, that had happened back when ->get_sb()
 went in - and hadn't been documented ;-/).  Just remove it from fs_flags
 (and see ->get_sb() entry for other actions).
+
+---
+[mandatory]
+
+->setattr() is called without BKL now.  Caller _always_ holds ->i_sem, so
+watch for ->i_sem-grabbing code that might be used by your ->setattr().
+Callers of notify_change() need ->i_sem now.

diff --git a/fs/attr.c b/fs/attr.c
index 7909632a4704dbf73c732f9debb0678e4216c236..f065ba5653390b04d1359c0a9680f31293ff1b86 100644 (file)
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -119,6 +119,7 @@ static int setattr_mask(unsigned int ia_valid)
 int notify_change(struct dentry * dentry, struct iattr * attr)
 {
        struct inode *inode = dentry->d_inode;
+       mode_t mode = inode->i_mode;
        int error;
        time_t now = CURRENT_TIME;
        unsigned int ia_valid = attr->ia_valid;
@@ -131,8 +132,25 @@ int notify_change(struct dentry * dentry, struct iattr * attr)
                attr->ia_atime = now;
        if (!(ia_valid & ATTR_MTIME_SET))
                attr->ia_mtime = now;
+       if (ia_valid & ATTR_KILL_SUID) {
+               if (mode & S_ISUID) {
+                       if (!ia_valid & ATTR_MODE) {
+                               ia_valid = attr->ia_valid |= ATTR_MODE;
+                               attr->ia_mode = inode->i_mode;
+                       }
+                       attr->ia_mode &= ~S_ISUID;
+               }
+       }
+       if (ia_valid & ATTR_KILL_SGID) {
+               if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
+                       if (!ia_valid & ATTR_MODE) {
+                               ia_valid = attr->ia_valid |= ATTR_MODE;
+                               attr->ia_mode = inode->i_mode;
+                       }
+                       attr->ia_mode &= ~S_ISGID;
+               }
+       }
 
-       down(&inode->i_sem);
        if (inode->i_op && inode->i_op->setattr) 
                error = inode->i_op->setattr(dentry, attr);
        else {
@@ -145,7 +163,6 @@ int notify_change(struct dentry * dentry, struct iattr * attr)
                                error = inode_setattr(inode, attr);
                }
        }
-       up(&inode->i_sem);
        if (!error) {
                unsigned long dn_mask = setattr_mask(ia_valid);
                if (dn_mask)

diff --git a/fs/dquot.c b/fs/dquot.c
index 81638f4b4a7df524f0e142fbe7eb2b1978e8fae2..20e663db70fa944d7c722cddb7a2fd99207a7a02 100644 (file)
--- a/fs/dquot.c
+++ b/fs/dquot.c
@@ -429,7 +429,7 @@ int shrink_dqcache_memory(int priority, unsigned int gfp_mask)
        count = nr_free_dquots / priority;
        prune_dqcache(count);
        unlock_kernel();
-       return kmem_cache_shrink_nr(dquot_cachep);
+       return kmem_cache_shrink(dquot_cachep);
 }
 
 /* NOTE: If you change this function please check whether dqput_blocks() works right... */

diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 724ca308f1a7a2025cb01f1ced2859eb7ed0f75e..57cd3df3c18a6d5669ff681edfe3dc3f0edfdb04 100644 (file)
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -1086,7 +1086,8 @@ int fat_notify_change(struct dentry * dentry, struct iattr * attr)
                        error = 0;
                goto out;
        }
-       if( error = inode_setattr(inode, attr) )
+       error = inode_setattr(inode, attr);
+       if (error)
                goto out;
 
        if (S_ISDIR(inode->i_mode))

diff --git a/fs/hpfs/namei.c b/fs/hpfs/namei.c
index 1e1c321bbeacf50c809e4aa4156ccd06a58d24cc..cacd39cac765a99058b87d1b4b39e6d28bba3b12 100644 (file)
--- a/fs/hpfs/namei.c
+++ b/fs/hpfs/namei.c
@@ -365,11 +365,9 @@ again:
                        goto ret;
                }
                /*printk("HPFS: truncating file before delete.\n");*/
-               down(&inode->i_sem);
                newattrs.ia_size = 0;
                newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
                err = notify_change(dentry, &newattrs);
-               up(&inode->i_sem);
                put_write_access(inode);
                if (err)
                        goto ret;

diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
index 404c8b63cb9ac4052903d09336f0a7b2b4b28cdc..97677034592f6d9f6ff25e35af7c9700cd2a21b7 100644 (file)
--- a/fs/jffs2/file.c
+++ b/fs/jffs2/file.c
@@ -43,6 +43,7 @@
 #include <linux/pagemap.h>
 #include <linux/crc32.h>
 #include <linux/jffs2.h>
+#include <linux/smp_lock.h>
 #include "nodelist.h"
 
 extern int generic_file_open(struct inode *, struct file *) __attribute__((weak));

diff --git a/fs/ncpfs/inode.c b/fs/ncpfs/inode.c
index d3cfbf5f929fe1c3c77c2ae1838518a11f2e7f9c..854b599fde90b2aac673857dba75bdfaa1b1c522 100644 (file)
--- a/fs/ncpfs/inode.c
+++ b/fs/ncpfs/inode.c
@@ -27,6 +27,7 @@
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/init.h>
+#include <linux/smp_lock.h>
 
 #include <linux/ncp_fs.h>
 

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index dc5232ebc20a832f957520bb19aba2211a810ed6..17a3867aaf269295a5545783e12054e737724354 100644 (file)
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -264,6 +264,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
                if (err)
                        goto out_nfserr;
 
+               size_change = 1;
                err = locks_verify_truncate(inode, NULL, iap->ia_size);
                if (err) {
                        put_write_access(inode);
@@ -279,35 +280,24 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
        }
 
        /* Revoke setuid/setgid bit on chown/chgrp */
-       if ((iap->ia_valid & ATTR_UID) && (imode & S_ISUID)
-        && iap->ia_uid != inode->i_uid) {
-               iap->ia_valid |= ATTR_MODE;
-               iap->ia_mode = imode &= ~S_ISUID;
-       }
-       if ((iap->ia_valid & ATTR_GID) && (imode & S_ISGID)
-        && iap->ia_gid != inode->i_gid) {
-               iap->ia_valid |= ATTR_MODE;
-               iap->ia_mode = imode &= ~S_ISGID;
-       }
+       if ((iap->ia_valid & ATTR_UID) && iap->ia_uid != inode->i_uid)
+               iap->ia_valid |= ATTR_KILL_SUID;
+       if ((iap->ia_valid & ATTR_GID) && iap->ia_gid != inode->i_gid)
+               iap->ia_valid |= ATTR_KILL_SGID;
 
        /* Change the attributes. */
 
-
        iap->ia_valid |= ATTR_CTIME;
 
-       if (iap->ia_valid & ATTR_SIZE) {
-               fh_lock(fhp);
-               size_change = 1;
-       }
        err = nfserr_notsync;
        if (!check_guard || guardtime == inode->i_ctime) {
+               fh_lock(fhp);
                err = notify_change(dentry, iap);
                err = nfserrno(err);
-       }
-       if (size_change) {
                fh_unlock(fhp);
-               put_write_access(inode);
        }
+       if (size_change)
+               put_write_access(inode);
        if (!err)
                if (EX_ISSYNC(fhp->fh_export))
                        write_inode_now(inode, 1);
@@ -725,10 +715,11 @@ nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t offset,
        /* clear setuid/setgid flag after write */
        if (err >= 0 && (inode->i_mode & (S_ISUID | S_ISGID))) {
                struct iattr    ia;
+               ia.ia_valid = ATTR_KILL_SUID | ATTR_KILL_SGID;
 
-               ia.ia_valid = ATTR_MODE;
-               ia.ia_mode  = inode->i_mode & ~(S_ISUID | S_ISGID);
+               down(&inode->i_sem);
                notify_change(dentry, &ia);
+               up(&inode->i_sem);
        }
 
        if (err >= 0 && stable) {
@@ -1157,7 +1148,9 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp,
                                iap->ia_valid |= ATTR_CTIME;
                                iap->ia_mode = (iap->ia_mode&S_IALLUGO)
                                        | S_IFLNK;
+                               down(&dentry->d_inode->i_sem);
                                err = notify_change(dnew, iap);
+                               up(&dentry->d_inode->i_sem);
                                if (!err && EX_ISSYNC(fhp->fh_export))
                                        write_inode_now(dentry->d_inode, 1);
                       }

diff --git a/fs/open.c b/fs/open.c
index 820fef8a609bcf3b8d6e104172666218e2122b92..32b010b7888ea1ee7115474317b8144fc74adf35 100644 (file)
--- a/fs/open.c
+++ b/fs/open.c
@@ -73,6 +73,7 @@ out:
 
 int do_truncate(struct dentry *dentry, loff_t length)
 {
+       int err;
        struct iattr newattrs;
 
        /* Not pretty: "inode->i_size" shouldn't really be signed. But it is. */
@@ -81,7 +82,10 @@ int do_truncate(struct dentry *dentry, loff_t length)
 
        newattrs.ia_size = length;
        newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
-       return notify_change(dentry, &newattrs);
+       down(&dentry->d_inode->i_sem);
+       err = notify_change(dentry, &newattrs);
+       up(&dentry->d_inode->i_sem);
+       return err;
 }
 
 static inline long do_sys_truncate(const char * path, loff_t length)
@@ -255,7 +259,9 @@ asmlinkage long sys_utime(char * filename, struct utimbuf * times)
                    (error = permission(inode,MAY_WRITE)) != 0)
                        goto dput_and_out;
        }
+       down(&inode->i_sem);
        error = notify_change(nd.dentry, &newattrs);
+       up(&inode->i_sem);
 dput_and_out:
        path_release(&nd);
 out:
@@ -299,7 +305,9 @@ asmlinkage long sys_utimes(char * filename, struct timeval * utimes)
                if ((error = permission(inode,MAY_WRITE)) != 0)
                        goto dput_and_out;
        }
+       down(&inode->i_sem);
        error = notify_change(nd.dentry, &newattrs);
+       up(&inode->i_sem);
 dput_and_out:
        path_release(&nd);
 out:
@@ -449,11 +457,13 @@ asmlinkage long sys_fchmod(unsigned int fd, mode_t mode)
        err = -EPERM;
        if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
                goto out_putf;
+       down(&inode->i_sem);
        if (mode == (mode_t) -1)
                mode = inode->i_mode;
        newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
        newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
        err = notify_change(dentry, &newattrs);
+       up(&inode->i_sem);
 
 out_putf:
        fput(file);
@@ -481,11 +491,13 @@ asmlinkage long sys_chmod(const char * filename, mode_t mode)
        if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
                goto dput_and_out;
 
+       down(&inode->i_sem);
        if (mode == (mode_t) -1)
                mode = inode->i_mode;
        newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
        newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
        error = notify_change(nd.dentry, &newattrs);
+       up(&inode->i_sem);
 
 dput_and_out:
        path_release(&nd);
@@ -510,45 +522,20 @@ static int chown_common(struct dentry * dentry, uid_t user, gid_t group)
        error = -EPERM;
        if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
                goto out;
-       if (user == (uid_t) -1)
-               user = inode->i_uid;
-       if (group == (gid_t) -1)
-               group = inode->i_gid;
-       newattrs.ia_mode = inode->i_mode;
-       newattrs.ia_uid = user;
-       newattrs.ia_gid = group;
-       newattrs.ia_valid =  ATTR_UID | ATTR_GID | ATTR_CTIME;
-       /*
-        * If the user or group of a non-directory has been changed by a
-        * non-root user, remove the setuid bit.
-        * 19981026     David C Niemi <niemi@tux.org>
-        *
-        * Changed this to apply to all users, including root, to avoid
-        * some races. This is the behavior we had in 2.0. The check for
-        * non-root was definitely wrong for 2.2 anyway, as it should
-        * have been using CAP_FSETID rather than fsuid -- 19990830 SD.
-        */
-       if ((inode->i_mode & S_ISUID) == S_ISUID &&
-               !S_ISDIR(inode->i_mode))
-       {
-               newattrs.ia_mode &= ~S_ISUID;
-               newattrs.ia_valid |= ATTR_MODE;
+       newattrs.ia_valid =  ATTR_CTIME;
+       if (user != (uid_t) -1) {
+               newattrs.ia_valid =  ATTR_UID;
+               newattrs.ia_uid = user;
        }
-       /*
-        * Likewise, if the user or group of a non-directory has been changed
-        * by a non-root user, remove the setgid bit UNLESS there is no group
-        * execute bit (this would be a file marked for mandatory locking).
-        * 19981026     David C Niemi <niemi@tux.org>
-        *
-        * Removed the fsuid check (see the comment above) -- 19990830 SD.
-        */
-       if (((inode->i_mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) 
-               && !S_ISDIR(inode->i_mode))
-       {
-               newattrs.ia_mode &= ~S_ISGID;
-               newattrs.ia_valid |= ATTR_MODE;
+       if (group != (gid_t) -1) {
+               newattrs.ia_valid =  ATTR_GID;
+               newattrs.ia_gid = group;
        }
+       if (!S_ISDIR(inode->i_mode))
+               newattrs.ia_valid |= ATTR_KILL_SUID|ATTR_KILL_SGID;
+       down(&inode->i_sem);
        error = notify_change(dentry, &newattrs);
+       up(&inode->i_sem);
 out:
        return error;
 }

diff --git a/include/linux/fs.h b/include/linux/fs.h
index dabeab042dd01c9d8dffd6794068ae7168e2ef3f..c27c3bc58cc93e49163c339bb3aec46fb6afb62a 100644 (file)
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -305,6 +305,8 @@ extern void set_bh_page(struct buffer_head *bh, struct page *page, unsigned long
 #define ATTR_MTIME_SET 256
 #define ATTR_FORCE     512     /* Not a change, but a change it */
 #define ATTR_ATTR_FLAG 1024
+#define ATTR_KILL_SUID 2048
+#define ATTR_KILL_SGID 4096
 
 /*
  * This is the Inode Attributes structure, used for notify_change().  It
@@ -1313,7 +1315,7 @@ static inline struct inode *iget(struct super_block *sb, unsigned long ino)
 
 extern void clear_inode(struct inode *);
 extern struct inode *new_inode(struct super_block *);
-extern void remove_suid(struct inode *inode);
+extern void remove_suid(struct dentry *);
 extern void insert_inode_hash(struct inode *);
 extern void remove_inode_hash(struct inode *);
 extern struct file * get_empty_filp(void);

diff --git a/mm/filemap.c b/mm/filemap.c
index 38567aa582b3caebf01314d03109e5780daa5872..ceb7df5fbe51952f006b12702de6dc014163d0e5 100644 (file)
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2503,18 +2503,19 @@ repeat:
        return page;
 }
 
-inline void remove_suid(struct inode *inode)
+inline void remove_suid(struct dentry *dentry)
 {
-       unsigned int mode;
+       struct iattr newattrs;
+       struct inode *inode = dentry->d_inode;
+       unsigned int mode = inode->i_mode & (S_ISUID|S_ISGID|S_IXGRP);
 
-       /* set S_IGID if S_IXGRP is set, and always set S_ISUID */
-       mode = (inode->i_mode & S_IXGRP)*(S_ISGID/S_IXGRP) | S_ISUID;
+       if (!(mode & S_IXGRP))
+               mode &= S_ISUID;
 
        /* was any of the uid bits set? */
-       mode &= inode->i_mode;
        if (mode && !capable(CAP_FSETID)) {
-               inode->i_mode &= ~mode;
-               mark_inode_dirty(inode);
+               newattrs.ia_valid = ATTR_KILL_SUID | ATTR_KILL_SGID;
+               notify_change(dentry, &newattrs);
        }
 }
 
@@ -2646,7 +2647,7 @@ generic_file_write(struct file *file,const char *buf,size_t count, loff_t *ppos)
        if (count == 0)
                goto out;
 
-       remove_suid(inode);
+       remove_suid(file->f_dentry);
        inode->i_ctime = inode->i_mtime = CURRENT_TIME;
        mark_inode_dirty_sync(inode);
 

diff --git a/mm/shmem.c b/mm/shmem.c
index bf477dd98f6e187d96c2aba6a1864e5eec26de11..a6e7093312da4679c2c6ee20c84615fe3b264204 100644 (file)
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -802,7 +802,7 @@ shmem_file_write(struct file *file,const char *buf,size_t count,loff_t *ppos)
 
        status  = 0;
        if (count) {
-               remove_suid(inode);
+               remove_suid(file->f_dentry);
                inode->i_ctime = inode->i_mtime = CURRENT_TIME;
        }