Re: Potential races around ->truncate()

[Anonymous]

On Mon, 21 Jun 1999, Linus Torvalds wrote:

> On Tue, 22 Jun 1999, Alexander Viro wrote:
> >
> >     knfsd calls ->truncate() without holding semaphore on inode.
> > Normally I would simply add obvious down()/up() there (and it is the
> > right thing for 2.2 anyway), but considering the stuff around
> > ->write()...
> >     Linus, do you plan to remove the semaphore holding for ->truncate()?
> > Then knfsd does the right thing and vmtruncate()/foofs_truncate()/do_truncate() 
> > need fixing. Comments?
> 
> I'll remove the semaphore around them in the 2.3.x series, but for 2.2.x
> the semaphore is definitely the right thing to do..

OK, then. Patch for 2.2.10 follows (semaphores + vmtruncate() that was
missing in nfsd_setattr()):

Index: vfs.c
===================================================================
RCS file: /home/al/projects/cvs/linux/2.2/fs/nfsd/vfs.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 vfs.c
--- vfs.c       1999/06/21 21:05:54     1.1.1.1
+++ vfs.c       1999/06/22 05:32:18
@@ -251,15 +251,6 @@
                err = get_write_access(inode);
                if (err)
                        goto out_nfserr;
-               /* N.B. Should we update the inode cache here? */
-               inode->i_size = iap->ia_size;
-               if (inode->i_op && inode->i_op->truncate)
-                       inode->i_op->truncate(inode);
-               mark_inode_dirty(inode);
-               put_write_access(inode);
-               iap->ia_valid &= ~ATTR_SIZE;
-               iap->ia_valid |= ATTR_MTIME;
-               iap->ia_mtime = CURRENT_TIME;
        }
 
        imode = inode->i_mode;
@@ -289,8 +280,20 @@
                if (current->fsuid != 0) {
                        saved_cap = current->cap_effective;
                        cap_clear(current->cap_effective);
+               }
+               if (iap->ia_valid & ATTR_SIZE)
+                       down(&inode->i_sem);
+                       err = notify_change(dentry, iap);
+                       if (!err) {
+                               vmtruncate(inode, iap->ia_size);
+                               if (inode->i_op && inode->i_op->truncate)
+                                       inode->i_op->truncate(inode);
+                       }
+                       up(&inode->i_sem);
+                       put_write_access(inode);
+               } else {
+                       err = notify_change(dentry, iap);
                }
-               err = notify_change(dentry, iap);
                if (current->fsuid != 0)
                        current->cap_effective = saved_cap;
                if (err)
@@ -781,14 +784,16 @@
                saved_cap = current->cap_effective;
                cap_clear(current->cap_effective);
        }
+       down(&inode->i_sem);
        err = notify_change(dentry, &newattrs);
-       if (current->fsuid != 0)
-               current->cap_effective = saved_cap;
        if (!err) {
                vmtruncate(inode, size);
                if (inode->i_op && inode->i_op->truncate)
                        inode->i_op->truncate(inode);
        }
+       up(&inode->i_sem);
+       if (current->fsuid != 0)
+               current->cap_effective = saved_cap;
        put_write_access(inode);
        DQUOT_DROP(inode);
        fh_unlock(fhp);