archive mirror
 help / color / mirror / Atom feed
From: Oleg Drokin <>
To: Andrew Morton <>
Subject: Re: [PATCH] [2.5] reiserfs: fix races between link and unlink on same file
Date: Fri, 1 Aug 2003 15:10:28 +0400	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>


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.


===== 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);
 	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);

  reply	other threads:[~2003-08-01 11:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-07-31 14:42 [PATCH] [2.5] reiserfs: fix races between link and unlink on same file Oleg Drokin
2003-07-31 20:37 ` Andrew Morton
2003-08-01 11:10   ` Oleg Drokin [this message]
2003-08-01 14:05     ` Chris Mason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).