linux-kernel - Re: [PATCH v2 9/9] fs/fuse: allow idmapped mounts

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: <20240814-knochen-ersparen-9b3f366caac4@brauner>
Date: Wed, 14 Aug 2024 16:19:12 +0200
From: Christian Brauner <brauner@...nel.org>
To: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
Cc: mszeredi@...hat.com, stgraber@...raber.org, 
	linux-fsdevel@...r.kernel.org, Seth Forshee <sforshee@...nel.org>, 
	Miklos Szeredi <miklos@...redi.hu>, Amir Goldstein <amir73il@...il.com>, 
	Bernd Schubert <bschubert@....com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 9/9] fs/fuse: allow idmapped mounts

On Wed, Aug 14, 2024 at 01:40:34PM GMT, Alexander Mikhalitsyn wrote:
> Now we have everything in place and we can allow idmapped mounts
> by setting the FS_ALLOW_IDMAP flag. Notice that real availability
> of idmapped mounts will depend on the fuse daemon. Fuse daemon
> have to set FUSE_ALLOW_IDMAP flag in the FUSE_INIT reply.
> 
> To discuss:
> - we enable idmapped mounts support only if "default_permissions" mode is enabled,
> because otherwise we would need to deal with UID/GID mappings in the userspace side OR
> provide the userspace with idmapped req->in.h.uid/req->in.h.gid values which is not
> something that we probably want to. Idmapped mounts phylosophy is not about faking
> caller uid/gid.
> 
> - We have a small offlist discussion with Christian around adding fs_type->allow_idmap
> hook. Christian pointed that it would be nice to have a superblock flag instead like
> SB_I_NOIDMAP and we can set this flag during mount time if we see that filesystem does not
> support idmappings. But, unfortunately I didn't succeed here because the kernel will
> know if the filesystem supports idmapping or not after FUSE_INIT request, but FUSE_INIT request
> is being sent at the end of mounting process, so mount and superblock will exist and
> visible by the userspace in that time. It seems like setting SB_I_NOIDMAP flag in this
> case is too late as user may do the trick with creating a idmapped mount while it wasn't
> restricted by SB_I_NOIDMAP. Alternatively, we can introduce a "positive" version SB_I_ALLOWIDMAP

Hm, I'm confused why won't the following (uncompiled) work?

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index ed4c2688047f..8ead1cacdd2f 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1346,10 +1346,12 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
                        if (flags & FUSE_OWNER_UID_GID_EXT)
                                fc->owner_uid_gid_ext = 1;
                        if (flags & FUSE_ALLOW_IDMAP) {
-                               if (fc->owner_uid_gid_ext && fc->default_permissions)
+                               if (fc->owner_uid_gid_ext && fc->default_permissions) {
                                        fc->allow_idmap = 1;
-                               else
+                                       fm->sb->s_iflags &= ~SB_I_NOIDMAP;
+                               } else {
                                        ok = false;
+                               }
                        }
                } else {
                        ra_pages = fc->max_read / PAGE_SIZE;
@@ -1576,6 +1578,7 @@ static void fuse_sb_defaults(struct super_block *sb)
        sb->s_time_gran = 1;
        sb->s_export_op = &fuse_export_operations;
        sb->s_iflags |= SB_I_IMA_UNVERIFIABLE_SIGNATURE;
+       sb->s_iflags |= SB_I_NOIDMAP;
        if (sb->s_user_ns != &init_user_ns)
                sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER;
        sb->s_flags &= ~(SB_NOSEC | SB_I_VERSION);
diff --git a/fs/namespace.c b/fs/namespace.c
index 328087a4df8a..d1702285c915 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -4436,6 +4436,10 @@ static int can_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
        if (!(m->mnt_sb->s_type->fs_flags & FS_ALLOW_IDMAP))
                return -EINVAL;

+       /* The filesystem has turned off idmapped mounts. */
+       if (m->mnt_sb->s_iflags & SB_I_NOIDMAP)
+               return -EINVAL;
+
        /* We're not controlling the superblock. */
        if (!ns_capable(fs_userns, CAP_SYS_ADMIN))
                return -EPERM;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index fd34b5755c0b..185004c41a5e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1189,6 +1189,7 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_I_TS_EXPIRY_WARNED 0x00000400 /* warned about timestamp range expiry */
 #define SB_I_RETIRED   0x00000800      /* superblock shouldn't be reused */
 #define SB_I_NOUMASK   0x00001000      /* VFS does not apply umask */
+#define SB_I_NOIDMAP   0x00002000      /* No idmapped mounts on this superblock */

 /* Possible states of 'frozen' field */
 enum {