[PATCH v5 2/3] fanotify: notify on mount attach and detach

[Paul Moore]

On Wed, Jan 29, 2025 at 11:58 AM Miklos Szeredi <mszeredi at redhat.com> wrote:
>
> Add notifications for attaching and detaching mounts.  The following new
> event masks are added:
>
>   FAN_MNT_ATTACH  - Mount was attached
>   FAN_MNT_DETACH  - Mount was detached
>
> If a mount is moved, then the event is reported with (FAN_MNT_ATTACH |
> FAN_MNT_DETACH).
>
> These events add an info record of type FAN_EVENT_INFO_TYPE_MNT containing
> these fields identifying the affected mounts:
>
>   __u64 mnt_id    - the ID of the mount (see statmount(2))
>
> FAN_REPORT_MNT must be supplied to fanotify_init() to receive these events
> and no other type of event can be received with this report type.
>
> Marks are added with FAN_MARK_MNTNS, which records the mount namespace from
> an nsfs file (e.g. /proc/self/ns/mnt).
>
> Signed-off-by: Miklos Szeredi <mszeredi at redhat.com>
> ---
>  fs/mount.h                         |  2 +
>  fs/namespace.c                     | 14 +++--
>  fs/notify/fanotify/fanotify.c      | 38 +++++++++++--
>  fs/notify/fanotify/fanotify.h      | 18 +++++++
>  fs/notify/fanotify/fanotify_user.c | 87 +++++++++++++++++++++++++-----
>  fs/notify/fdinfo.c                 |  5 ++
>  include/linux/fanotify.h           | 12 +++--
>  include/uapi/linux/fanotify.h      | 10 ++++
>  security/selinux/hooks.c           |  4 ++
>  9 files changed, 167 insertions(+), 23 deletions(-)

...

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 7b867dfec88b..06d073eab53c 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3395,6 +3395,10 @@ static int selinux_path_notify(const struct path *path, u64 mask,
>         case FSNOTIFY_OBJ_TYPE_INODE:
>                 perm = FILE__WATCH;
>                 break;
> +       case FSNOTIFY_OBJ_TYPE_MNTNS:
> +               /* Maybe introduce FILE__WATCH_MOUNTNS? */
> +               perm = FILE__WATCH_MOUNT;
> +               break;
>         default:
>                 return -EINVAL;
>         }

Ignoring for a moment that this patch was merged without an explicit
ACK for the SELinux changes, let's talk about these SELinux changes
...

I understand that you went with the "simpler version" because you
didn't believe the discussion was converging, which is fair, however,
I believe Daniel's argument is convincing enough to warrant the new
permission.  Yes, it has taken me approximately two days to find the
time to revisit this topic and reply with some clarity, but personally
I feel like that is not an unreasonable period of time, especially for
a new feature discussion occurring during the merge window.

If you need an example on how to add a new SELinux permission, you can
look at commit ed5d44d42c95 ("selinux: Implement userns_create hook")
for a fairly simple example.  In the watch_mountns case things are
slightly different due to the existence of the COMMON_FILE_PERMS
macro, but you basically want to add "watch_mountns" to the end of the
COMMON_FILE_PERMS macro in security/selinux/include/classmap.h.  Of
course if you aren't sure about something, let me know.  As a FYI, my
network access will be spotty starting tonight and extending through
the weekend, but I will have network/mail access at least once a day.

Now back to the merge into the VFS tree ... I was very surprised to
open this patchset and see that Christian had merged v5 after less
than 24 hours (at least according to the email timestamps that I see)
and without an explicit ACK for the SELinux changes.  I've mentioned
this to you before Christian, please do not merge any SELinux, LSM
framework, or audit related patches without an explicit ACK.  I
recognize that sometimes there are highly critical security issues
that need immediate attention, but that is clearly not the case here,
and there are other procedures to help deal with those emergency
scenarios.

-- 
paul-moore.com