Thread (11 messages) 11 messages, 5 authors, 2025-03-14

Re: [RFC PATCH 1/6] fs: invoke LSM file_open hook in do_dentry_open for O_PATH fds as well

From: Christian Brauner <brauner@kernel.org>
Date: 2025-03-13 08:50:43
Also in: linux-fsdevel, lkml, selinux

On Wed, Mar 12, 2025 at 09:37:14PM +0000, Al Viro wrote:
On Wed, Mar 12, 2025 at 02:21:41PM -0700, Ryan Lee wrote:
quoted
Currently, opening O_PATH file descriptors completely bypasses the LSM
infrastructure. Invoking the LSM file_open hook for O_PATH fds will
be necessary for e.g. mediating the fsmount() syscall.
LSM mediation for the mount api should be done by adding appropriate
hooks to the new mount api.
quoted
Signed-off-by: Ryan Lee <redacted>
---
 fs/open.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/open.c b/fs/open.c
index 30bfcddd505d..0f8542bf6cd4 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -921,8 +921,13 @@ static int do_dentry_open(struct file *f,
 	if (unlikely(f->f_flags & O_PATH)) {
 		f->f_mode = FMODE_PATH | FMODE_OPENED;
 		file_set_fsnotify_mode(f, FMODE_NONOTIFY);
 		f->f_op = &empty_fops;
-		return 0;
+		/*
+		 * do_o_path in fs/namei.c unconditionally invokes path_put
+		 * after this function returns, so don't path_put the path
+		 * upon LSM rejection of O_PATH opening
+		 */
+		return security_file_open(f);
Unconditional path_put() in do_o_path() has nothing to do with that -
what gets dropped there is the reference acquired there; the reference
acquired (and not dropped) here is the one that went into ->f_path.
Since you are leaving FMODE_OPENED set, you will have __fput() drop
that reference.

Basically, you are simulating behaviour on the O_DIRECT open of
something that does not support O_DIRECT - return an error, with
->f_path and FMODE_OPENED left in place.

Said that, what I do not understand is the point of that exercise -
why does LSM need to veto anything for those and why is security_file_open()
I really think this is misguided. This should be done via proper hooks
into apis that use O_PATH file descriptors for specific purposes but not
for the generic open() path.
the right place for such checks?
It isn't.
The second part is particularly interesting...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help