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