Re: [PATCH v2] vfs: fix link vs. rename race

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

On Tue, Sep 13, 2022 at 06:20:10AM +0100, Al Viro wrote:

> > Alternately, lock the "from" directory as well as the "to" directory.
> > That would mean using lock_rename() and generally copying the structure
> > of do_renameat2() into do_linkat()
> 
> Ever done cp -al?  Cross-directory renames are relatively rare; cross-directory
> links can be fairly heavy on some payloads, and you'll get ->s_vfs_rename_mutex
> held a _lot_.
> 
> > I wonder if you could get a similar race trying to create a file in
> > (empty directory) /tmp/foo while /tmp/bar was being renamed over it.
> 
> 	Neil, no offense, but... if you have plans regarding changes in
> directory locking, you might consider reading through the file called
> Documentation/filesystems/directory-locking.rst
> 
> 	Occasionally documentation is where one could expect to find it...

... and that "..." above should've been ";-)" - it was not intended as
a dig, especially since locking in that area has subtle and badly
underdocumented parts (in particular, anything related to fh_to_dentry(),
rules regarding the stability of ->d_name and ->d_parent, mount vs. dentry
invalidation and too many other things), but the basic stuff like that
is actually covered.

FWIW, the answer to your question is that the victim of overwriting
rename is held locked by caller of ->rename(); combined with the lock
held on directory by anyone who modifies it that prevents the race
you are asking about.

See
        if (!is_dir || (flags & RENAME_EXCHANGE))
                lock_two_nondirectories(source, target);
        else if (target)
                inode_lock(target);
in vfs_rename().