On Fri, Oct 11, 2024 at 11:42 PM Paul Moore [off-list ref] wrote:

On Oct 11, 2024 Song Liu [off-list ref] wrote:

quoted

Currently, fsnotify_open_perm() is called from security_file_open(). This
is not right for CONFIG_SECURITY=n and CONFIG_FSNOTIFY=y case, as
security_file_open() in this combination will be a no-op and not call
fsnotify_open_perm(). Fix this by calling fsnotify_open_perm() directly.

Signed-off-by: Song Liu <song@kernel.org>
---
PS: I didn't included a Fixes tag. This issue was probably introduced 15
years ago in [1]. If we want to back port this to stable, we will need
another version for older kernel because of [2].

[1] c4ec54b40d33 ("fsnotify: new fsnotify hooks and events types for access decisions")
[2] 36e28c42187c ("fsnotify: split fsnotify_perm() into two hooks")
---
 fs/open.c           | 4 ++++
 security/security.c | 9 +--------
 2 files changed, 5 insertions(+), 8 deletions(-)

Nice cleanup, but please finish off the coupling of lsm/fsnotify altogether.
I would either change the title to "decouple fsnotify from lsm" or
submit an additional patch with that title.

diff --git a/fs/notify/fanotify/Kconfig b/fs/notify/fanotify/Kconfig
index a511f9d8677b..0e36aaf379b7 100644
--- a/fs/notify/fanotify/Kconfig
+++ b/fs/notify/fanotify/Kconfig

@@ -15,7 +15,6 @@ config FANOTIFY
 config FANOTIFY_ACCESS_PERMISSIONS
        bool "fanotify permissions checking"
        depends on FANOTIFY
-       depends on SECURITY
        default n
        help
           Say Y here is you want fanotify listeners to be able to

make permissions

diff --git a/security/security.c b/security/security.c
index 6875eb4a59fc..8d238ffdeb4a 100644
--- a/security/security.c
+++ b/security/security.c

@@ -19,7 +19,6 @@
 #include <linux/kernel.h>
 #include <linux/kernel_read_file.h>
 #include <linux/lsm_hooks.h>
-#include <linux/fsnotify.h>
 #include <linux/mman.h>
 #include <linux/mount.h>
 #include <linux/personality.h>

This looks fine to me, if we can get an ACK from the VFS folks I can
merge this into the lsm/stable-6.12 tree and send it to Linus, or the
VFS folks can do it if they prefer (my ACK is below just in case).

My preference would be to take this via the vfs or fsnotify tree.

As far as stable prior to v6.8 is concerned, once this hits Linus'
tree you can submit an adjusted backport for the older kernels to the
stable team.

Please do NOT submit an adjustable backport.
Instead please include the following tags for the decoupling patch:

Depends-on: 36e28c42187c fsnotify: split fsnotify_perm() into two hooks
Depends-on: d9e5d31084b0 fsnotify: optionally pass access range in
file permission hooks

Acked-by: Paul Moore <paul@paul-moore.com>

Thanks,
Amir.