From: Christian Brauner <brauner@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org, Josef Bacik <josef@toxicpanda.com>,
Jann Horn <jannh@google.com>, Jeff Layton <jlayton@kernel.org>,
Jan Kara <jack@suse.com>,
Daan De Meyer <daan.j.demeyer@gmail.com>,
Christian Brauner <brauner@kernel.org>
Subject: [PATCH RFC 0/2] Add an fcntl() to check file creation
Date: Wed, 24 Jul 2024 15:15:34 +0200 [thread overview]
Message-ID: <20240724-work-fcntl-v1-0-e8153a2f1991@kernel.org> (raw)
Systemd has a helper called openat_report_new() that returns whether a
file was created anew or it already existed before for cases where
O_CREAT has to be used without O_EXCL (cf. [1]). That apparently isn't
something that's specific to systemd but it's where I noticed it.
The current logic is that it first attempts to open the file without
O_CREAT | O_EXCL and if it gets ENOENT the helper tries again with both
flags. If that succeeds all is well. If it now reports EEXIST it
retries.
That works fairly well but some corner cases make this more involved. If
this operates on a dangling symlink the first openat() without O_CREAT |
O_EXCL will return ENOENT but the second openat() with O_CREAT | O_EXCL
will fail with EEXIST. The reason is that openat() without O_CREAT |
O_EXCL follows the symlink while O_CREAT | O_EXCL doesn't for security
reasons. So it's not something we can really change unless we add an
explicit opt-in via O_FOLLOW which seems really ugly.
The caller could try and use fanotify() to register to listen for
creation events in the directory before calling openat(). The caller
could then compare the returned tid to its own tid to ensure that even
in threaded environments it actually created the file. That might work
but is a lot of work for something that should be fairly simple and I'm
uncertain about it's reliability.
The caller could use a bpf lsm hook to hook into security_file_open() to
figure out whether they created the file. That also seems a bit wild.
So let's add F_CREATED which allows the caller to check whether they
actually did create the file. That has caveats of course but I don't
think they are problematic:
* In multi-threaded environments a thread can only be sure that it did
create the file if it calls openat() with O_CREAT. In other words,
it's obviously not enough to just go through it's fdtable and check
these fds because another thread could've created the file.
* If there's any codepaths where an openat() with O_CREAT would yield
the same struct file as that of another thread it would obviously
cause wrong results. I'm not aware of any such codepaths from openat()
itself. Imho, that would be a bug.
* Related to the previous point, calling the new fcntl() on files created
and opened via special-purpose system calls or ioctl()s would cause
wrong results only if the affected subsystem a) raises FMODE_CREATED
and b) may return the same struct file for two different calls. I'm
not seeing anything outside of regular VFS code that raises
FMODE_CREATED.
There is code for b) in e.g., the drm layer where the same struct file
is resurfaced but again FMODE_CREATED isn't used and it would be very
misleading if it did.
[1]: https://github.com/systemd/systemd/blob/11d5e2b5fbf9f6bfa5763fd45b56829ad4f0777f/src/basic/fs-util.c#L1078
Thanks!
Christian
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
---
base-commit: 933069701c1b507825b514317d4edd5d3fd9d417
change-id: 20240724-work-fcntl-9d2aa275affd
next reply other threads:[~2024-07-24 13:16 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-24 13:15 Christian Brauner [this message]
2024-07-24 13:15 ` [PATCH RFC 1/2] fcntl: add F_CREATED Christian Brauner
2024-07-24 13:56 ` Jeff Layton
2024-07-25 8:38 ` Christian Brauner
2024-07-25 11:16 ` Jeff Layton
2024-07-24 16:10 ` Jan Kara
2024-07-24 13:15 ` [PATCH RFC 2/2] selftests: add F_CREATED tests Christian Brauner
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=20240724-work-fcntl-v1-0-e8153a2f1991@kernel.org \
--to=brauner@kernel.org \
--cc=daan.j.demeyer@gmail.com \
--cc=jack@suse.com \
--cc=jannh@google.com \
--cc=jlayton@kernel.org \
--cc=josef@toxicpanda.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).