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

[Christian Brauner]

On Thu, Jan 30, 2025 at 04:05:53PM -0500, Paul Moore wrote:
> 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

Things go into the tree for testing when the VFS side is ready for
testing. We're at v5 and the patchset has gone through four iterations
over multiple months. It will go into linux-next and fs-next now for as
much expsure as possible.

I'm not sure what the confusion between merging things into a tree and
sending things upstream is. I have explained this to you before. The
application message is also pretty clear about that.

> 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