Re: [PATCH] [2.5] reiserfs: fix races between link and unlink on same file

From: Oleg Drokin (green@namesys.com)
Date: Fri Aug 01 2003 - 06:10:28 EST


Hello!

On Thu, Jul 31, 2003 at 01:37:08PM -0700, Andrew Morton wrote:

> > This patch (originally by Chris Mason) fixes link/unlink races in reiserfs.
> Could you describe the race a little more please? Why is the VFS's hold of
> i_sem on the parent directory not sufficient?

Well, we do not take i_sem on parent directory of source filename for sys_link, I think.
So we might endup in sys_link() with inode that is already deleted/being deleted (and nlink==0).
Actually, I naturally thought that only i_nlink check to be non zero at reiserfs_link time should be
enough, but Chris is sure that we need entire patch, so may be he may add more comments to that.

BTW, looking at vfs_link, this patch (below the message) seems to be natural thing to do, is not it?

> > +
> > + /*
> > + * we schedule before doing the add_save_link call, save the link
> > + * count so we don't race
> This comment would seem to imply lock_kernel()-based locking, but
> lock_kernel() is not held here.

It is.
This reiserfs_write_lock/unlock stuff are in fact lock_kernel()/unlock_kernel(),
only I invented those macroses to easy conversion to more fine-grained locking later.
Except that Hans now does not want this to happen.

Bye,
    Oleg

===== fs/namei.c 1.81 vs edited =====
--- 1.81/fs/namei.c Fri Jul 11 09:23:45 2003
+++ edited/fs/namei.c Fri Aug 1 14:40:29 2003
@@ -1793,17 +1793,17 @@
                 return -EPERM;
         if (!dir->i_op || !dir->i_op->link)
                 return -EPERM;
- if (S_ISDIR(old_dentry->d_inode->i_mode))
+ if (S_ISDIR(inode->i_mode))
                 return -EPERM;
 
         error = security_inode_link(old_dentry, dir, new_dentry);
         if (error)
                 return error;
 
- down(&old_dentry->d_inode->i_sem);
+ down(&inode->i_sem);
         DQUOT_INIT(dir);
         error = dir->i_op->link(old_dentry, dir, new_dentry);
- up(&old_dentry->d_inode->i_sem);
+ up(&inode->i_sem);
         if (!error) {
                 inode_dir_notify(dir, DN_CREATE);
                 security_inode_post_link(old_dentry, dir, new_dentry);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Aug 07 2003 - 22:00:16 EST