All of lore.kernel.org
 help / color / mirror / Atom feed
From: "NeilBrown" <neilb@suse.de>
To: "Christian Brauner" <brauner@kernel.org>
Cc: "Amir Goldstein" <amir73il@gmail.com>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Miklos Szeredi" <mszeredi@redhat.com>,
	"Xavier Roche" <xavier.roche@algolia.com>,
	"linux-fsdevel" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH RFC] VFS: lock source directory for link to avoid rename race.
Date: Tue, 20 Sep 2022 08:56:42 +1000	[thread overview]
Message-ID: <166362820252.9160.14622721614019063947@noble.neil.brown.name> (raw)
In-Reply-To: <20220919082848.s7dwk4hwxxejwm7s@wittgenstein>

On Mon, 19 Sep 2022, Christian Brauner wrote:
> On Fri, Sep 16, 2022 at 05:32:45PM +0300, Amir Goldstein wrote:
> > On Fri, Sep 16, 2022 at 9:26 AM NeilBrown <neilb@suse.de> wrote:
> > >
> > >
> > > rename(2) is documented as
> > >
> > >        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.
> > >
> > > However link(2) from a given path can race with rename renaming to that
> > > path so that link gets -ENOENT because the path has already been unlinked
> > > by rename, and creating a link to an unlinked file is not permitted.
> > >
> > 
> > I have to ask. Is this a real problem or just a matter of respecting
> > the laws of this man page?
> 
> I have to say that I have the same reaction. The commit message doesn't
> really explain where the current behavior becomes an issue and whether
> there are any users seeing issues with this. And the patch makes
> do_linkat() way more complex than it was before.
> 

A bug is a bug .... and in this case it is an intriguing puzzle too.

Yes, the commit message could say a bit more about context.

The patch also isn't correct, so the complexity is not relevant in this
case.  Some complexity will likely been needed (I do have a really
simple patch that just retries the whole op, but I don't think that is
safe), and we do need to balance the complexity against the value.
Ideally we could end up making the code simpler ...  I'm not sure I can
manage that though :-)

Thanks,
NeilBrown

  reply	other threads:[~2022-09-19 22:57 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-21  8:20 [PATCH v2] vfs: fix link vs. rename race Miklos Szeredi
2022-02-21  8:56 ` Xavier Roche
2022-09-13  2:04 ` Al Viro
2022-09-13  4:29   ` Al Viro
2022-09-13  8:02     ` Amir Goldstein
2022-09-13 10:03       ` Miklos Szeredi
2022-09-13  4:41 ` NeilBrown
2022-09-13  5:20   ` Al Viro
2022-09-13  5:40     ` Al Viro
2022-09-14  0:14       ` NeilBrown
2022-09-14  1:30         ` Al Viro
2022-09-13 23:52     ` NeilBrown
2022-09-14  0:13       ` Al Viro
2022-09-16  6:13         ` [PATCH RFC] VFS: lock source directory for link to avoid " NeilBrown
2022-09-16  6:28           ` Miklos Szeredi
2022-09-16  6:45             ` NeilBrown
2022-09-16  6:49             ` Al Viro
2022-09-16 14:32           ` Amir Goldstein
2022-09-19  8:28             ` Christian Brauner
2022-09-19 22:56               ` NeilBrown [this message]
2022-09-23  3:02           ` [VFS] 3fb4ec6faa: ltp.linkat02.fail kernel test robot
2022-09-23  3:02             ` kernel test robot
2022-09-23  3:02             ` [LTP] " kernel test robot

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=166362820252.9160.14622721614019063947@noble.neil.brown.name \
    --to=neilb@suse.de \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mszeredi@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=xavier.roche@algolia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.