linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: linux-fsdevel@vger.kernel.org
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	Amir Goldstein <amir73il@gmail.com>,
	Chuck Lever <chuck.lever@oracle.com>,
	Namjae Jeon <linkinjeon@kernel.org>,
	John Johansen <john@apparmor.net>
Subject: [PATCH 2/2] Have cc(1) catch attempts to modify ->f_path
Date: Sat, 6 Sep 2025 10:14:58 +0100	[thread overview]
Message-ID: <20250906091458.GC31600@ZenIV> (raw)
In-Reply-To: <20250906090738.GA31600@ZenIV>

[last one in #work.f_path, following the merge with #work.mount and #work.path]

There are very few places that have cause to do that - all in core
VFS now, and all done to files that are not yet opened (or visible
to anybody else, for that matter).

Let's turn f_path into a union of struct path __f_path and const
struct path f_path.  It's C, not C++ - 6.5.2.3[4] in C99 and
later explicitly allows that kind of type-punning.

That way any attempts to bypass these checks will be either very
easy to catch, or (if the bastards get sufficiently creative to
make it hard to spot with grep alone) very clearly malicious -
and still catchable with a bit of instrumentation for sparse.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/file_table.c b/fs/file_table.c
index 85b53e39138d..b223d873e48b 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -171,7 +171,7 @@ static int init_file(struct file *f, int flags, const struct cred *cred)
 	 * the respective member when opening the file.
 	 */
 	mutex_init(&f->f_pos_lock);
-	memset(&f->f_path, 0, sizeof(f->f_path));
+	memset(&f->__f_path, 0, sizeof(f->f_path));
 	memset(&f->f_ra, 0, sizeof(f->f_ra));
 
 	f->f_flags	= flags;
@@ -319,7 +319,7 @@ struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
 static void file_init_path(struct file *file, const struct path *path,
 			   const struct file_operations *fop)
 {
-	file->f_path = *path;
+	file->__f_path = *path;
 	file->f_inode = path->dentry->d_inode;
 	file->f_mapping = path->dentry->d_inode->i_mapping;
 	file->f_wb_err = filemap_sample_wb_err(file->f_mapping);
diff --git a/fs/namei.c b/fs/namei.c
index 3eb0408e3400..ba8bf73d2f9c 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3563,8 +3563,8 @@ static struct dentry *atomic_open(struct nameidata *nd, struct dentry *dentry,
 	if (nd->flags & LOOKUP_DIRECTORY)
 		open_flag |= O_DIRECTORY;
 
-	file->f_path.dentry = DENTRY_NOT_SET;
-	file->f_path.mnt = nd->path.mnt;
+	file->__f_path.dentry = DENTRY_NOT_SET;
+	file->__f_path.mnt = nd->path.mnt;
 	error = dir->i_op->atomic_open(dir, dentry, file,
 				       open_to_namei_flags(open_flag), mode);
 	d_lookup_done(dentry);
@@ -3932,8 +3932,8 @@ int vfs_tmpfile(struct mnt_idmap *idmap,
 	child = d_alloc(parentpath->dentry, &slash_name);
 	if (unlikely(!child))
 		return -ENOMEM;
-	file->f_path.mnt = parentpath->mnt;
-	file->f_path.dentry = child;
+	file->__f_path.mnt = parentpath->mnt;
+	file->__f_path.dentry = child;
 	mode = vfs_prepare_mode(idmap, dir, mode, mode, mode);
 	error = dir->i_op->tmpfile(idmap, dir, file, mode);
 	dput(child);
diff --git a/fs/open.c b/fs/open.c
index 9655158c3885..f4bdf7693530 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1022,8 +1022,8 @@ static int do_dentry_open(struct file *f,
 	put_file_access(f);
 cleanup_file:
 	path_put(&f->f_path);
-	f->f_path.mnt = NULL;
-	f->f_path.dentry = NULL;
+	f->__f_path.mnt = NULL;
+	f->__f_path.dentry = NULL;
 	f->f_inode = NULL;
 	return error;
 }
@@ -1050,7 +1050,7 @@ int finish_open(struct file *file, struct dentry *dentry,
 {
 	BUG_ON(file->f_mode & FMODE_OPENED); /* once it's opened, it's opened */
 
-	file->f_path.dentry = dentry;
+	file->__f_path.dentry = dentry;
 	return do_dentry_open(file, open);
 }
 EXPORT_SYMBOL(finish_open);
@@ -1071,7 +1071,7 @@ EXPORT_SYMBOL(finish_open);
  */
 int finish_no_open(struct file *file, struct dentry *dentry)
 {
-	file->f_path.dentry = dentry;
+	file->__f_path.dentry = dentry;
 	return 0;
 }
 EXPORT_SYMBOL(finish_no_open);
@@ -1091,7 +1091,7 @@ int vfs_open(const struct path *path, struct file *file)
 {
 	int ret;
 
-	file->f_path = *path;
+	file->__f_path = *path;
 	ret = do_dentry_open(file, NULL);
 	if (!ret) {
 		/*
diff --git a/include/linux/fs.h b/include/linux/fs.h
index af514fae4e2d..7fe4831b7663 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1107,7 +1107,10 @@ struct file {
 	const struct cred		*f_cred;
 	struct fown_struct		*f_owner;
 	/* --- cacheline 1 boundary (64 bytes) --- */
-	struct path			f_path;
+	union {
+		const struct path	f_path;
+		struct path		__f_path;
+	};
 	union {
 		/* regular files (with FMODE_ATOMIC_POS) and directories */
 		struct mutex		f_pos_lock;

  parent reply	other threads:[~2025-09-06  9:15 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` Al Viro [this message]
2025-09-08  9:52   ` [PATCH 2/2] Have cc(1) catch attempts to modify ->f_path 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

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=20250906091458.GC31600@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=jack@suse.cz \
    --cc=john@apparmor.net \
    --cc=linkinjeon@kernel.org \
    --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).