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

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

On Tue, Sep 13, 2022 at 02:41:51PM +1000, NeilBrown wrote:
> On Mon, 21 Feb 2022, Miklos Szeredi wrote:
> > There has been a longstanding race condition between rename(2) and link(2),
> > when those operations are done in parallel:
> > 
> > 1. Moving a file to an existing target file (eg. mv file target)
> > 2. Creating a link from the target file to a third file (eg. ln target
> >    link)
> > 
> > By the time vfs_link() locks the target inode, it might already be unlinked
> > by rename.  This results in vfs_link() returning -ENOENT in order to
> > prevent linking to already unlinked files.  This check was introduced in
> > v2.6.39 by commit aae8a97d3ec3 ("fs: Don't allow to create hardlink for
> > deleted file").
> > 
> > This breaks apparent atomicity of rename(2), which is described in
> > standards and the man page:
> > 
> >     "If newpath already exists, it will be atomically replaced, so that
> >      there is no point at which another process attempting to access
> >      newpath will find it missing."
> > 
> > The simplest fix is to exclude renames for the complete link operation.
> 
> 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...