On Thu, Apr 20, 2023 at 4:12 PM Jan Kara <jack@xxxxxxx> wrote: > > On Sun 16-04-23 09:07:22, Amir Goldstein wrote: > > An unprivileged user is allowed to create an fanotify group and add > > inode marks, but not filesystem and mount marks. > > > > Add limited support for setting up filesystem and mount marks by an > > unprivileged user under the following conditions: > > > > 1. User has CAP_SYS_ADMIN in the user ns where the group was created > > 2.a. User has CAP_SYS_ADMIN in the user ns where the filesystem was > > mounted (implies FS_USERNS_MOUNT) > > OR (in case setting up a mark mount) > > 2.b. User has CAP_SYS_ADMIN in the user ns attached to an idmapped mount > > > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > > The patch looks good to me. Just two comments below. > > > Christian, > > > > You can find this patch, along with FAN_UNMOUNT patches on my github [3]. > > Please confirm that this meets your needs for watching container mounts. > > > > [3] https://github.com/amir73il/linux/commits/fan_unmount > > Yeah, it would be good to get ack from Christian that the model you propose > works for him. > > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > > index db3b79b8e901..2c3e123aee14 100644 > > --- a/fs/notify/fanotify/fanotify_user.c > > +++ b/fs/notify/fanotify/fanotify_user.c > > @@ -1238,6 +1238,7 @@ static struct fsnotify_mark *fanotify_add_new_mark(struct fsnotify_group *group, > > * A group with FAN_UNLIMITED_MARKS does not contribute to mark count > > * in the limited groups account. > > */ > > + BUILD_BUG_ON(!(FANOTIFY_ADMIN_INIT_FLAGS & FAN_UNLIMITED_MARKS)); > > if (!FAN_GROUP_FLAG(group, FAN_UNLIMITED_MARKS) && > > !inc_ucount(ucounts->ns, ucounts->uid, UCOUNT_FANOTIFY_MARKS)) > > return ERR_PTR(-ENOSPC); > ... > > @@ -1557,21 +1559,13 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags) > > goto out_destroy_group; > > } > > > > + BUILD_BUG_ON(!(FANOTIFY_ADMIN_INIT_FLAGS & FAN_UNLIMITED_QUEUE)); > > if (flags & FAN_UNLIMITED_QUEUE) { > > - fd = -EPERM; > > - if (!capable(CAP_SYS_ADMIN)) > > - goto out_destroy_group; > > group->max_events = UINT_MAX; > > } else { > > group->max_events = fanotify_max_queued_events; > > } > > > > - if (flags & FAN_UNLIMITED_MARKS) { > > - fd = -EPERM; > > - if (!capable(CAP_SYS_ADMIN)) > > - goto out_destroy_group; > > - } > > - > > Perhaps this hunk (plus the BUILD_BUG_ON hunk above) should go into a > separate patch with a proper changelog? I was scratching my head over it > for a while until I've realized it's unrelated cleanup of dead code. > Sure. It took me a while to understand it myself, when I re-read it after two years... Thanks, Amir.
