Re: [RFC][PATCH] fanotify: support watching filesystems and mounts inside userns
From: Jan Kara <jack@suse.cz>
Date: 2023-04-20 13:12:21
Also in:
linux-fsdevel
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@gmail.com>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.
quoted hunk ↗ jump to hunk
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);
...
quoted hunk ↗ jump to hunk
@@ -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. Honza -- Jan Kara [off-list ref] SUSE Labs, CR