stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] fanotify: fix permission model of unprivileged group" failed to apply to 5.12-stable tree
@ 2021-06-08 12:21 gregkh
  2021-06-08 12:33 ` Amir Goldstein
  0 siblings, 1 reply; 2+ messages in thread
From: gregkh @ 2021-06-08 12:21 UTC (permalink / raw)
  To: amir73il, Stable, christian.brauner, jack, repnop; +Cc: stable


The patch below does not apply to the 5.12-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From a8b98c808eab3ec8f1b5a64be967b0f4af4cae43 Mon Sep 17 00:00:00 2001
From: Amir Goldstein <amir73il@gmail.com>
Date: Mon, 24 May 2021 16:53:21 +0300
Subject: [PATCH] fanotify: fix permission model of unprivileged group

Reporting event->pid should depend on the privileges of the user that
initialized the group, not the privileges of the user reading the
events.

Use an internal group flag FANOTIFY_UNPRIV to record the fact that the
group was initialized by an unprivileged user.

To be on the safe side, the premissions to setup filesystem and mount
marks now require that both the user that initialized the group and
the user setting up the mark have CAP_SYS_ADMIN.

Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxiA77_P5vtv7e83g0+9d7B5W9ZTE4GfQEYbWmfT1rA=VA@mail.gmail.com/
Fixes: 7cea2a3c505e ("fanotify: support limited functionality for unprivileged users")
Cc: <Stable@vger.kernel.org> # v5.12+
Link: https://lore.kernel.org/r/20210524135321.2190062-1-amir73il@gmail.com
Reviewed-by: Matthew Bobrowski <repnop@google.com>
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 71fefb30e015..be5b6d2c01e7 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -424,11 +424,18 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
 	 * events generated by the listener process itself, without disclosing
 	 * the pids of other processes.
 	 */
-	if (!capable(CAP_SYS_ADMIN) &&
+	if (FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) &&
 	    task_tgid(current) != event->pid)
 		metadata.pid = 0;
 
-	if (path && path->mnt && path->dentry) {
+	/*
+	 * For now, fid mode is required for an unprivileged listener and
+	 * fid mode does not report fd in events.  Keep this check anyway
+	 * for safety in case fid mode requirement is relaxed in the future
+	 * to allow unprivileged listener to get events with no fd and no fid.
+	 */
+	if (!FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) &&
+	    path && path->mnt && path->dentry) {
 		fd = create_fd(group, path, &f);
 		if (fd < 0)
 			return fd;
@@ -1040,6 +1047,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 	int f_flags, fd;
 	unsigned int fid_mode = flags & FANOTIFY_FID_BITS;
 	unsigned int class = flags & FANOTIFY_CLASS_BITS;
+	unsigned int internal_flags = 0;
 
 	pr_debug("%s: flags=%x event_f_flags=%x\n",
 		 __func__, flags, event_f_flags);
@@ -1053,6 +1061,13 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 		 */
 		if ((flags & FANOTIFY_ADMIN_INIT_FLAGS) || !fid_mode)
 			return -EPERM;
+
+		/*
+		 * Setting the internal flag FANOTIFY_UNPRIV on the group
+		 * prevents setting mount/filesystem marks on this group and
+		 * prevents reporting pid and open fd in events.
+		 */
+		internal_flags |= FANOTIFY_UNPRIV;
 	}
 
 #ifdef CONFIG_AUDITSYSCALL
@@ -1105,7 +1120,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 		goto out_destroy_group;
 	}
 
-	group->fanotify_data.flags = flags;
+	group->fanotify_data.flags = flags | internal_flags;
 	group->memcg = get_mem_cgroup_from_mm(current->mm);
 
 	group->fanotify_data.merge_hash = fanotify_alloc_merge_hash();
@@ -1305,11 +1320,13 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 	group = f.file->private_data;
 
 	/*
-	 * An unprivileged user is not allowed to watch a mount point nor
-	 * a filesystem.
+	 * An unprivileged user is not allowed to setup mount nor filesystem
+	 * marks.  This also includes setting up such marks by a group that
+	 * was initialized by an unprivileged user.
 	 */
 	ret = -EPERM;
-	if (!capable(CAP_SYS_ADMIN) &&
+	if ((!capable(CAP_SYS_ADMIN) ||
+	     FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV)) &&
 	    mark_type != FAN_MARK_INODE)
 		goto fput_and_out;
 
@@ -1460,6 +1477,7 @@ static int __init fanotify_user_setup(void)
 	max_marks = clamp(max_marks, FANOTIFY_OLD_DEFAULT_MAX_MARKS,
 				     FANOTIFY_DEFAULT_MAX_USER_MARKS);
 
+	BUILD_BUG_ON(FANOTIFY_INIT_FLAGS & FANOTIFY_INTERNAL_GROUP_FLAGS);
 	BUILD_BUG_ON(HWEIGHT32(FANOTIFY_INIT_FLAGS) != 10);
 	BUILD_BUG_ON(HWEIGHT32(FANOTIFY_MARK_FLAGS) != 9);
 
diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c
index a712b2aaa9ac..57f0d5d9f934 100644
--- a/fs/notify/fdinfo.c
+++ b/fs/notify/fdinfo.c
@@ -144,7 +144,7 @@ void fanotify_show_fdinfo(struct seq_file *m, struct file *f)
 	struct fsnotify_group *group = f->private_data;
 
 	seq_printf(m, "fanotify flags:%x event-flags:%x\n",
-		   group->fanotify_data.flags,
+		   group->fanotify_data.flags & FANOTIFY_INIT_FLAGS,
 		   group->fanotify_data.f_flags);
 
 	show_fdinfo(m, f, fanotify_fdinfo);
diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
index bad41bcb25df..a16dbeced152 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -51,6 +51,10 @@ extern struct ctl_table fanotify_table[]; /* for sysctl */
 #define FANOTIFY_INIT_FLAGS	(FANOTIFY_ADMIN_INIT_FLAGS | \
 				 FANOTIFY_USER_INIT_FLAGS)
 
+/* Internal group flags */
+#define FANOTIFY_UNPRIV		0x80000000
+#define FANOTIFY_INTERNAL_GROUP_FLAGS	(FANOTIFY_UNPRIV)
+
 #define FANOTIFY_MARK_TYPE_BITS	(FAN_MARK_INODE | FAN_MARK_MOUNT | \
 				 FAN_MARK_FILESYSTEM)
 


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: FAILED: patch "[PATCH] fanotify: fix permission model of unprivileged group" failed to apply to 5.12-stable tree
  2021-06-08 12:21 FAILED: patch "[PATCH] fanotify: fix permission model of unprivileged group" failed to apply to 5.12-stable tree gregkh
@ 2021-06-08 12:33 ` Amir Goldstein
  0 siblings, 0 replies; 2+ messages in thread
From: Amir Goldstein @ 2021-06-08 12:33 UTC (permalink / raw)
  To: Greg KH; +Cc: stable, Christian Brauner, Jan Kara, Matthew Bobrowski

On Tue, Jun 8, 2021 at 3:21 PM <gregkh@linuxfoundation.org> wrote:
>
>
> The patch below does not apply to the 5.12-stable tree.

I wrongly tagged this # v5.12+
It fixed a commit merge in this cycle.

Sorry for the noise.

Thanks,
Amir.

> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
>
> thanks,
>
> greg k-h
>
> ------------------ original commit in Linus's tree ------------------
>
> From a8b98c808eab3ec8f1b5a64be967b0f4af4cae43 Mon Sep 17 00:00:00 2001
> From: Amir Goldstein <amir73il@gmail.com>
> Date: Mon, 24 May 2021 16:53:21 +0300
> Subject: [PATCH] fanotify: fix permission model of unprivileged group
>
> Reporting event->pid should depend on the privileges of the user that
> initialized the group, not the privileges of the user reading the
> events.
>
> Use an internal group flag FANOTIFY_UNPRIV to record the fact that the
> group was initialized by an unprivileged user.
>
> To be on the safe side, the premissions to setup filesystem and mount
> marks now require that both the user that initialized the group and
> the user setting up the mark have CAP_SYS_ADMIN.
>
> Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxiA77_P5vtv7e83g0+9d7B5W9ZTE4GfQEYbWmfT1rA=VA@mail.gmail.com/
> Fixes: 7cea2a3c505e ("fanotify: support limited functionality for unprivileged users")
> Cc: <Stable@vger.kernel.org> # v5.12+
> Link: https://lore.kernel.org/r/20210524135321.2190062-1-amir73il@gmail.com
> Reviewed-by: Matthew Bobrowski <repnop@google.com>
> Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 71fefb30e015..be5b6d2c01e7 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -424,11 +424,18 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>          * events generated by the listener process itself, without disclosing
>          * the pids of other processes.
>          */
> -       if (!capable(CAP_SYS_ADMIN) &&
> +       if (FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) &&
>             task_tgid(current) != event->pid)
>                 metadata.pid = 0;
>
> -       if (path && path->mnt && path->dentry) {
> +       /*
> +        * For now, fid mode is required for an unprivileged listener and
> +        * fid mode does not report fd in events.  Keep this check anyway
> +        * for safety in case fid mode requirement is relaxed in the future
> +        * to allow unprivileged listener to get events with no fd and no fid.
> +        */
> +       if (!FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) &&
> +           path && path->mnt && path->dentry) {
>                 fd = create_fd(group, path, &f);
>                 if (fd < 0)
>                         return fd;
> @@ -1040,6 +1047,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>         int f_flags, fd;
>         unsigned int fid_mode = flags & FANOTIFY_FID_BITS;
>         unsigned int class = flags & FANOTIFY_CLASS_BITS;
> +       unsigned int internal_flags = 0;
>
>         pr_debug("%s: flags=%x event_f_flags=%x\n",
>                  __func__, flags, event_f_flags);
> @@ -1053,6 +1061,13 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>                  */
>                 if ((flags & FANOTIFY_ADMIN_INIT_FLAGS) || !fid_mode)
>                         return -EPERM;
> +
> +               /*
> +                * Setting the internal flag FANOTIFY_UNPRIV on the group
> +                * prevents setting mount/filesystem marks on this group and
> +                * prevents reporting pid and open fd in events.
> +                */
> +               internal_flags |= FANOTIFY_UNPRIV;
>         }
>
>  #ifdef CONFIG_AUDITSYSCALL
> @@ -1105,7 +1120,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>                 goto out_destroy_group;
>         }
>
> -       group->fanotify_data.flags = flags;
> +       group->fanotify_data.flags = flags | internal_flags;
>         group->memcg = get_mem_cgroup_from_mm(current->mm);
>
>         group->fanotify_data.merge_hash = fanotify_alloc_merge_hash();
> @@ -1305,11 +1320,13 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>         group = f.file->private_data;
>
>         /*
> -        * An unprivileged user is not allowed to watch a mount point nor
> -        * a filesystem.
> +        * An unprivileged user is not allowed to setup mount nor filesystem
> +        * marks.  This also includes setting up such marks by a group that
> +        * was initialized by an unprivileged user.
>          */
>         ret = -EPERM;
> -       if (!capable(CAP_SYS_ADMIN) &&
> +       if ((!capable(CAP_SYS_ADMIN) ||
> +            FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV)) &&
>             mark_type != FAN_MARK_INODE)
>                 goto fput_and_out;
>
> @@ -1460,6 +1477,7 @@ static int __init fanotify_user_setup(void)
>         max_marks = clamp(max_marks, FANOTIFY_OLD_DEFAULT_MAX_MARKS,
>                                      FANOTIFY_DEFAULT_MAX_USER_MARKS);
>
> +       BUILD_BUG_ON(FANOTIFY_INIT_FLAGS & FANOTIFY_INTERNAL_GROUP_FLAGS);
>         BUILD_BUG_ON(HWEIGHT32(FANOTIFY_INIT_FLAGS) != 10);
>         BUILD_BUG_ON(HWEIGHT32(FANOTIFY_MARK_FLAGS) != 9);
>
> diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c
> index a712b2aaa9ac..57f0d5d9f934 100644
> --- a/fs/notify/fdinfo.c
> +++ b/fs/notify/fdinfo.c
> @@ -144,7 +144,7 @@ void fanotify_show_fdinfo(struct seq_file *m, struct file *f)
>         struct fsnotify_group *group = f->private_data;
>
>         seq_printf(m, "fanotify flags:%x event-flags:%x\n",
> -                  group->fanotify_data.flags,
> +                  group->fanotify_data.flags & FANOTIFY_INIT_FLAGS,
>                    group->fanotify_data.f_flags);
>
>         show_fdinfo(m, f, fanotify_fdinfo);
> diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
> index bad41bcb25df..a16dbeced152 100644
> --- a/include/linux/fanotify.h
> +++ b/include/linux/fanotify.h
> @@ -51,6 +51,10 @@ extern struct ctl_table fanotify_table[]; /* for sysctl */
>  #define FANOTIFY_INIT_FLAGS    (FANOTIFY_ADMIN_INIT_FLAGS | \
>                                  FANOTIFY_USER_INIT_FLAGS)
>
> +/* Internal group flags */
> +#define FANOTIFY_UNPRIV                0x80000000
> +#define FANOTIFY_INTERNAL_GROUP_FLAGS  (FANOTIFY_UNPRIV)
> +
>  #define FANOTIFY_MARK_TYPE_BITS        (FAN_MARK_INODE | FAN_MARK_MOUNT | \
>                                  FAN_MARK_FILESYSTEM)
>
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-06-08 12:34 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-08 12:21 FAILED: patch "[PATCH] fanotify: fix permission model of unprivileged group" failed to apply to 5.12-stable tree gregkh
2021-06-08 12:33 ` Amir Goldstein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).