linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHES] file->f_path safety and struct path constifications
@ 2025-09-06  9:07 Al Viro
  2025-09-06  9:11 ` [PATCH 01/21] backing_file_user_path(): constify struct path * Al Viro
                   ` (3 more replies)
  0 siblings, 4 replies; 70+ messages in thread
From: Al Viro @ 2025-09-06  9:07 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Linus Torvalds, Christian Brauner, Jan Kara, Amir Goldstein,
	Chuck Lever, Namjae Jeon, John Johansen

	struct path is embedded into several objects, starting with
struct file.  In a lot of places we rely upon e.g. file->f_path of an
opened file remaining unchanging; things like "ask for write access to
file->f_path.mnt when opening for write, release that on final fput()"
would break badly if ->f_path.mnt could change between those.  It's not
the only place like that; both VFS and filesystems expect that to hold.
Anything that would want to play silly buggers with that would have to be
_very_ careful and would be rather brittle, at that.  It's not impossible
to get away with, but any such place is a source of headache for proofs
of correctness, as well as a likely cause of bugs in the future.

	Unfortunately, verifying that turns into several hours of manual
audit that has to be repeated once in a while and I'm sick and tired of
doing that.  Let the compiler deal with that crap.  The same goes for
struct unix_sock ->path, etc.

Note that in the mainline we have _very_ few places that store to ->f_path.
	1) init_file() zeroes it out
	2) file_init_path() copies the caller-supplied struct path into it
(common helper of alloc_file() family of primitives; struct file is freshly
allocated, we are setting it up)
	3) atomic_open()/finish_open()/finish_no_open() arrange for setting
->f_path between them.  Again, that's before it gets opened.
	4) vfs_tmpfile() - ditto.
	5) do_dentry_open() clears it on early failure exits
	6) vfs_open() sets it to caller-supplied struct path - that's opening
the file by path (dentry_open() and its ilk are using that one).  Again,
prior to file getting opened.
	7) acct_on() (acct(2) helper) is flipping ->f_path.mnt of its internally
opened and internally used file to cloned mount.  It does get away with that,
but it's neither pretty nor robust.

	All except the last one are in core VFS and not dealing with
already opened files.  Killing (7) is doable - it's not hard to get rid
of that weird shit in acct_on().  After that no stores happen to opened
files and all those stores are local to fs/file_table.c, fs/open.c and
fs/namei.c (the latter - in the open-related parts).

	After that the obvious next step would be to turn f_path into
type-punning union of struct path __f_path and const struct path
f_path, and switch the places that should do stores (see above) to
using ->__f_path.  It's valid C99 - no UB there.  struct file no longer
can be a modifiable lvalue, but we never did wholesale copying for those
and there's no reason to start; what's more, we never embed struct file
into any other objects.

	It's not quite that simple, though - things like
	return vfs_statx_path(&fd_file(f)->f_path, flags, stat, request_mask);
would have the compiler complain; it needs to be told that vfs_statx_path()
is not going to modify the struct path it's been given.  IOW, we need to
switch a bunch of struct path * arguments to const struct path * before we
can go for the final part.  Turns out that there's not a lot of such
missing annotations.

So this stuff sits in two branches:

#work.path switches the struct path * arguments that are never used to modify
the struct path in question to const struct path *.  Not all of those
functions are used for ->f_path, but there's not a lot of them and new call
sites might appear, so let's deal with all that bunch.

#work.f_path starts with getting rid of the shit in acct_on(), then merges
#work.path and #work.mount in (I don't want to pull constification patches
out of #work.mount), then does the conversion of f_path to anon union.

Branches are in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.path and
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.f_path resp.;
individual patches in followups.

Please, review.  If nobody objects, I'm putting that into #for-next early
next week...

^ permalink raw reply	[flat|nested] 70+ messages in thread

end of thread, other threads:[~2025-09-26  9:13 UTC | newest]

Thread overview: 70+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-06  9:07 [PATCHES] file->f_path safety and struct path constifications Al Viro
2025-09-06  9:11 ` [PATCH 01/21] backing_file_user_path(): constify struct path * Al Viro
2025-09-06  9:11   ` [PATCH 02/21] constify path argument of vfs_statx_path() Al Viro
2025-09-08  9:38     ` Jan Kara
2025-09-15 12:01     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 03/21] filename_lookup(): constify root argument Al Viro
2025-09-08  9:39     ` Jan Kara
2025-09-15 12:00     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 04/21] done_path_create(): constify path argument Al Viro
2025-09-08  9:40     ` Jan Kara
2025-09-15 12:01     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 05/21] bpf...d_path(): " Al Viro
2025-09-08  9:41     ` Jan Kara
2025-09-15 12:01     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 06/21] nfs: constify path argument of __vfs_getattr() Al Viro
2025-09-08  9:41     ` Jan Kara
2025-09-15 12:02     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 07/21] rqst_exp_get_by_name(): constify path argument Al Viro
2025-09-06 15:16     ` Chuck Lever
2025-09-15 12:02     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 08/21] export_operations->open(): " Al Viro
2025-09-08  9:42     ` Jan Kara
2025-09-15 12:03     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 09/21] check_export(): " Al Viro
2025-09-08  9:43     ` Jan Kara
2025-09-15 12:03     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 10/21] ksmbd_vfs_path_lookup_locked(): root_share_path can be const struct path * Al Viro
2025-09-07  1:45     ` Namjae Jeon
2025-09-15 12:03     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 11/21] ksmbd_vfs_kern_path_unlock(): constify path argument Al Viro
2025-09-07  1:44     ` Namjae Jeon
2025-09-15 12:04     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 12/21] ksmbd_vfs_inherit_posix_acl(): " Al Viro
2025-09-07  1:44     ` Namjae Jeon
2025-09-15 12:04     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 13/21] ksmbd_vfs_set_init_posix_acl(): " Al Viro
2025-09-07  1:44     ` Namjae Jeon
2025-09-15 12:04     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 14/21] ovl_ensure_verity_loaded(): constify datapath argument Al Viro
2025-09-15 12:04     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 15/21] ovl_validate_verity(): constify {meta,data}path arguments Al Viro
2025-09-15 12:05     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 16/21] ovl_get_verity_digest(): constify path argument Al Viro
2025-09-15 12:05     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 17/21] ovl_lower_dir(): " Al Viro
2025-09-15 12:05     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 18/21] ovl_sync_file(): " Al Viro
2025-09-15 12:05     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 19/21] ovl_is_real_file: constify realpath argument Al Viro
2025-09-15 12:06     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 20/21] apparmor/af_unix: constify struct path * arguments Al Viro
2025-09-08  9:46     ` Jan Kara
2025-09-15 12:06     ` Christian Brauner
2025-09-06  9:11   ` [PATCH 21/21] configfs:get_target() - release path as soon as we grab configfs_item reference Al Viro
2025-09-08  9:46     ` Jan Kara
2025-09-15 12:07     ` Christian Brauner
2025-09-08  9:38   ` [PATCH 01/21] backing_file_user_path(): constify struct path * Jan Kara
2025-09-15 11:59   ` Christian Brauner
2025-09-06  9:13 ` [PATCH 1/2] kernel/acct.c: saner struct file treatment Al Viro
2025-09-25 11:40   ` Mark Brown
2025-09-25 12:28     ` Jan Kara
2025-09-25 18:56       ` Al Viro
2025-09-25 19:09         ` Al Viro
2025-09-25 19:36           ` Al Viro
2025-09-25 20:56           ` Al Viro
2025-09-26  9:13             ` Jan Kara
2025-09-06  9:14 ` [PATCH 2/2] Have cc(1) catch attempts to modify ->f_path Al Viro
2025-09-08  9:52   ` Jan Kara
2025-09-15 12:00   ` Christian Brauner
2025-09-06  9:16 ` [PATCHES] file->f_path safety and struct path constifications Al Viro

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).