Re: [RFC PATCH] fs: introduce mkdirat2 syscall for atomic mkdir

From: "Drew DeVault" <sir@cmpwn.com>
To: "Al Viro" <viro@zeniv.linux.org.uk>
Cc: <linux-fsdevel@vger.kernel.org>, <linux-api@vger.kernel.org>,
	"Aleksa Sarai" <cyphar@cyphar.com>
Subject: Re: [RFC PATCH] fs: introduce mkdirat2 syscall for atomic mkdir
Date: Sun, 28 Feb 2021 08:56:04 -0500	[thread overview]
Message-ID: <C9L7RW0S7YU0.16I8160PKEP0K@taiga> (raw)
In-Reply-To: <YDsGzhBzLzSp6nPj@zeniv-ca.linux.org.uk>

On Sat Feb 27, 2021 at 9:58 PM EST, Al Viro wrote:
> open() *always* returns descriptor or an error, for one thing.
> And quite a few of open() flags are completely wrong for mkdir,
> starting with symlink following and truncation.

So does mkdirat2. Are you referring to the do_mkdirat2 function? I
merged mkdir/mkdirat/mkdirat2 into one function with a flag to enable
the mkdirat2 behavior, to avoid copying and pasting much of the
functionality. However, the syscalls themselves don't overload their
return value as you expect. mkdir & mkdirat both still return 0 or an
error, and mkdirat2 always returns an fd or an error. If you prefer, I
can leave their implementations separate so that this is more clear.

I supposed the flags might be wrong - should I just introduce a new set
of flags, with the specific ones which are useful (which I think is just
O_CLOEXEC)?

> What's more, your implementation is both racy and deadlock-prone -
> it repeats the entire pathwalk with no warranty that it'll
> arrive to the object you've created *AND* if you have
> something like /foo/bar/baz/../../splat and dentry of bar
> gets evicted on memory pressure, that pathwalk will end up
> trying to look bar up. In the already locked /foo, aka
> /foo/bar/baz/../..

This is down to unfamiliarity with this code, I think. I'll try to give
it a closer look.

> TBH, I don't understand what are you trying to achieve -
> what will that mkdir+open combination buy you, especially
> since that atomicity goes straight out of window if you try
> to use that on e.g. NFS. How is the userland supposed to make
> use of that thing?

I'm trying to close what appears to be an oversight in the API. See the
previous threads:

https://lore.kernel.org/linux-fsdevel/C9KKYZ4T5O53.338Y48UIQ9W3H@taiga/T/#t
https://lore.kernel.org/linux-fsdevel/20200316142057.xo24zea3k5zwswra@yavin/

Userland uses it the same way they use mkdir+open, but in one call, so
that they can use the directory they make as soon as it's created. The
atomicity goal, if possible, would also add a reference to the new
directory via the open fd, so they can use it even if it's removed by
another process. It makes such applications less error-prone, albiet in
a minor edge case.

I'm not sure what's involved with the NFS case, but I can look into it.