Re: A missing i_mutex in rename? (Linux kernel 2.6.latest)

[Al Viro <viro@ftp.linux.org.uk>]

On Wed, Apr 19, 2006 at 01:51:21PM +0100, Anton Altaparmakov wrote:
> >  - I don't immediately see a race that taking the lock on the victim of
> >    sys_unlink() solves; however, for symmetry with sys_rmdir(), it seems
> >    desirable.
> 
> I guess the symmetry thing is fair enough.

Not only; it does, among other things, guarantee that fs can assume that
->link() won't race with unlink() (and that link count is protected by
->i_mutex, while we are at it).

> >  - sys_link() needs to lock the target to be sure it isn't removed and
> >    replaced with a directory in the meantime.
> 
> Agreed.

"Replaced" part is bogus - we'd done lookup, so we won't get anything new.

> > If you need to lock the old inode inside ntfs for your own consistency
> > purposes, that looks like it should be fine, but the VFS doesn't need to
> > lock it for you.
> 
> Great, thanks.  That was my own conclusion also but it never hurts to be 
> sure.  (-:
> 
> ntfs_rename() at the moment looks roughly like this:
> 
> if (target_inode) {
> 	if (S_ISDIR(target_inode->i_mode)
> 		ntfs_rmdir(target_dir_inode, target_dentry);
> 	else
> 		ntfs_unlink(target_dir_inode, target_dentry);
> }
> mutex_lock(&old_inode->i_mutex);
> ntfs_link(old_dentry, target_dir_inode, target_dentry);
> ntfs_unlink(old_dir_inode, old_dentry);
> mutex_unlock(&old_inode->i_mutex);

Have fun dealing with error handling in the above...  Note that failing
rename() should _NOT_ lead to target disappearing.