linux-kernel - Re: [PATCH] fs: don't allow non-init s_user_ns for filesystems without FS_USERNS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJqdLrr78NzGkUzYhCiFJmvWZDYbCFaeJ19XHwFh2VK2J4+VvA@mail.gmail.com>
Date: Wed, 24 Jul 2024 17:17:20 +0200
From: Alexander Mikhalitsyn <alexander@...alicyn.com>
To: "Seth Forshee (DigitalOcean)" <sforshee@...nel.org>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, 
	Amir Goldstein <amir73il@...il.com>, Aleksa Sarai <cyphar@...har.com>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fs: don't allow non-init s_user_ns for filesystems
 without FS_USERNS_MOUNT

Am Mi., 24. Juli 2024 um 16:54 Uhr schrieb Seth Forshee (DigitalOcean)
<sforshee@...nel.org>:
>
> Christian noticed that it is possible for a privileged user to mount
> most filesystems with a non-initial user namespace in sb->s_user_ns.
> When fsopen() is called in a non-init namespace the caller's namespace
> is recorded in fs_context->user_ns. If the returned file descriptor is
> then passed to a process priviliged in init_user_ns, that process can
> call fsconfig(fd_fs, FSCONFIG_CMD_CREATE), creating a new superblock
> with sb->s_user_ns set to the namespace of the process which called
> fsopen().
>
> This is problematic. We cannot assume that any filesystem which does not
> set FS_USERNS_MOUNT has been written with a non-initial s_user_ns in
> mind, increasing the risk for bugs and security issues.
>
> Prevent this by returning EPERM from sget_fc() when FS_USERNS_MOUNT is
> not set for the filesystem and a non-initial user namespace will be
> used. sget() does not need to be updated as it always uses the user
> namespace of the current context, or the initial user namespace if
> SB_SUBMOUNT is set.
>
> Fixes: cb50b348c71f ("convenience helpers: vfs_get_super() and sget_fc()")
> Reported-by: Christian Brauner <brauner@...nel.org>
> Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@...nel.org>

Hi Seth!

LGTM

Reviewed-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>

> ---
>  fs/super.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/fs/super.c b/fs/super.c
> index 095ba793e10c..d681fb7698d8 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -736,6 +736,17 @@ struct super_block *sget_fc(struct fs_context *fc,
>         struct user_namespace *user_ns = fc->global ? &init_user_ns : fc->user_ns;
>         int err;
>
> +       /*
> +        * Never allow s_user_ns != &init_user_ns when FS_USERNS_MOUNT is
> +        * not set, as the filesystem is likely unprepared to handle it.
> +        * This can happen when fsconfig() is called from init_user_ns with
> +        * an fs_fd opened in another user namespace.
> +        */
> +       if (user_ns != &init_user_ns && !(fc->fs_type->fs_flags & FS_USERNS_MOUNT)) {
> +               errorfc(fc, "mounting from non-initial user namespace is not allowed");
> +               return ERR_PTR(-EPERM);
> +       }
> +
>  retry:
>         spin_lock(&sb_lock);
>         if (test) {
>
> ---
> base-commit: 256abd8e550ce977b728be79a74e1729438b4948
> change-id: 20240723-s_user_ns-fix-b00c31de1cb8
>
> Best regards,
> --
> Seth Forshee (DigitalOcean) <sforshee@...nel.org>
>