linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/15] fuse: basic support for idmapped mounts
@ 2024-09-03 15:16 Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 01/15] fs/namespace: introduce SB_I_NOIDMAP flag Alexander Mikhalitsyn
                   ` (15 more replies)
  0 siblings, 16 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Vivek Goyal, German Maglione, Amir Goldstein, Bernd Schubert,
	Alexander Mikhalitsyn, linux-kernel

Dear friends,

This patch series aimed to provide support for idmapped mounts
for fuse & virtiofs. We already have idmapped mounts support for almost all
widely-used filesystems:
* local (ext4, btrfs, xfs, fat, vfat, ntfs3, squashfs, f2fs, erofs, ZFS (out-of-tree))
* network (ceph)

Git tree (based on torvalds/master):
v4: https://github.com/mihalicyn/linux/commits/fuse_idmapped_mounts.v4
current: https://github.com/mihalicyn/linux/commits/fuse_idmapped_mounts

Changelog for version 4:
- heavily reworked to comply with Miklos's suggestion to start sending idmapped uid/gid
  just in fuse header instead of adding a new FUSE_OWNER_UID_GID_EXT extension (please, refer to [6], [7])
- added ("fs/fuse: handle idmappings properly in ->write_iter")
- added ("fs/fuse: warn if fuse_access is called when idmapped mounts are allowed")
- added handling for idmapped mounts in FUSE_EXT_GROUPS extension
- now RENAME_WHITEOUT can be (and is) supported

Changelog for version 3:
- introduce and use a new SB_I_NOIDMAP flag (suggested by Christian)
- add support for virtiofs (+user space virtiofsd conversion)

Changelog for version 2:
- removed "fs/namespace: introduce fs_type->allow_idmap hook" and simplified logic
to return -EIO if a fuse daemon does not support idmapped mounts (suggested
by Christian Brauner)
- passed an "idmap" in more cases even when it's not necessary to simplify things (suggested
by Christian Brauner)
- take ->rename() RENAME_WHITEOUT into account and forbid it for idmapped mount case

Links to previous versions:
v3: https://lore.kernel.org/all/20240815092429.103356-1-aleksandr.mikhalitsyn@canonical.com
tree: https://github.com/mihalicyn/linux/commits/fuse_idmapped_mounts.v3
v2: https://lore.kernel.org/linux-fsdevel/20240814114034.113953-1-aleksandr.mikhalitsyn@canonical.com
tree: https://github.com/mihalicyn/linux/commits/fuse_idmapped_mounts.v2
v1: https://lore.kernel.org/all/20240108120824.122178-1-aleksandr.mikhalitsyn@canonical.com/#r
tree: https://github.com/mihalicyn/linux/commits/fuse_idmapped_mounts.v1

Having fuse (+virtiofs) supported looks like a good next step. At the same time
fuse conceptually close to the network filesystems and supporting it is
a quite challenging task.

Let me briefly explain what was done in this series and which obstacles we have.

With this series, you can use idmapped mounts with fuse if the following
conditions are met:
1. The filesystem daemon declares idmap support (new FUSE_INIT response feature
flag FUSE_ALLOW_IDMAP)
2. The filesystem superblock was mounted with the "default_permissions" parameter
3. The filesystem fuse daemon does not perform any UID/GID-based checks internally
and fully trusts the kernel to do that (yes, it's almost the same as 2.)

I have prepared a bunch of real-world examples of the user space modifications
that can be done to use this extension:
- libfuse support
https://github.com/mihalicyn/libfuse/commits/idmap_support
- fuse-overlayfs support:
https://github.com/mihalicyn/fuse-overlayfs/commits/idmap_support
- cephfs-fuse conversion example
https://github.com/mihalicyn/ceph/commits/fuse_idmap
- glusterfs conversion example (there is a conceptual issue)
https://github.com/mihalicyn/glusterfs/commits/fuse_idmap
- virtiofsd conversion example
https://gitlab.com/virtio-fs/virtiofsd/-/merge_requests/245

The glusterfs is a bit problematic, unfortunately, because even if the glusterfs
superblock was mounted with the "default_permissions" parameter (1 and 2 conditions
are satisfied), it fails to satisfy the 3rd condition. The glusterfs fuse daemon sends
caller UIDs/GIDs over the wire and all the permission checks are done twice (first
on the client side (in the fuse kernel module) and second on the glusterfs server side).
Just for demonstration's sake, I found a hacky (but working) solution for glusterfs
that disables these server-side permission checks (see [1]). This allows you to play
with the filesystem and idmapped mounts and it works just fine.

The problem described above is the main problem that we can meet when
working on idmapped mounts support for network-based filesystems (or network-like filesystems
like fuse). When people look at the idmapped mounts feature at first they tend to think
that idmaps are for faking caller UIDs/GIDs, but that's not the case. There was a big
discussion about this in the "ceph: support idmapped mounts" patch series [2], [3].
The brief outcome from this discussion is that we don't want and don't have to fool
filesystem code and map a caller's UID/GID everywhere, but only in VFS i_op's
which are provided with a "struct mnt_idmap *idmap"). For example ->lookup()
callback is not provided with it and that's on purpose! We don't expect the low-level
filesystem code to do any permissions checks inside this callback because everything
was already checked on the higher level (see may_lookup() helper). For local filesystems
this assumption works like a charm, but for network-based, unfortunately, not.
For example, the cephfs kernel client *always* send called UID/GID with *any* request
(->lookup included!) and then *may* (depending on the MDS configuration) perform any
permissions checks on the server side based on these values, which obviously leads
to issues/inconsistencies if VFS idmaps are involved.

Fuse filesystem very-very close to cephfs example, because we have req->in.h.uid/req->in.h.gid
and these values are present in all fuse requests and userspace may use them as it wants.

All of the above explains why we have a "default_permissions" requirement. If filesystem
does not use it, then permission checks will be widespread across all the i_op's like
->lookup, ->unlink, ->readlink instead of being consolidated in the one place (->permission callback).

How to play with it:
1. take any patched filesystem from the list (fuse-overlayfs, cephfs-fuse, glusterfs) and mount it
2. ./mount-idmapped --map-mount b:1000:0:2 /mnt/my_fuse_mount /mnt/my_fuse_mount_idmapped
(maps UID/GIDs as 1000 -> 0, 1001 -> 1)
[ taken from https://raw.githubusercontent.com/brauner/mount-idmapped/master/mount-idmapped.c ]

[1] https://github.com/mihalicyn/glusterfs/commit/ab3ec2c7cbe22618cba9cc94a52a492b1904d0b2
[2] https://lore.kernel.org/lkml/20230608154256.562906-1-aleksandr.mikhalitsyn@canonical.com/
[3] https://lore.kernel.org/lkml/CAEivzxfw1fHO2TFA4dx3u23ZKK6Q+EThfzuibrhA3RKM=ZOYLg@mail.gmail.com/
[4] https://github.com/ceph/ceph/pull/52575
[5] https://lore.kernel.org/all/20230807132626.182101-4-aleksandr.mikhalitsyn@canonical.com/
[6] https://lore.kernel.org/all/CAJfpegsVY97_5mHSc06mSw79FehFWtoXT=hhTUK_E-Yhr7OAuQ@mail.gmail.com/
[7] https://lore.kernel.org/all/CAJfpegtHQsEUuFq1k4ZbTD3E1h-GsrN3PWyv7X8cg6sfU_W2Yw@mail.gmail.com/


Thanks!
Alex

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: German Maglione <gmaglione@redhat.com>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>

Alexander Mikhalitsyn (15):
  fs/namespace: introduce SB_I_NOIDMAP flag
  fs/fuse: add basic infrastructure to support idmappings
  fs/fuse: add an idmap argument to fuse_simple_request
  fs/fuse: support idmapped FUSE_EXT_GROUPS
  fs/fuse: support idmap for mkdir/mknod/symlink/create/tmpfile
  fs/fuse: support idmapped getattr inode op
  fs/fuse: support idmapped ->permission inode op
  fs/fuse: support idmapped ->setattr op
  fs/fuse: drop idmap argument from __fuse_get_acl
  fs/fuse: support idmapped ->set_acl
  fs/fuse: support idmapped ->rename op
  fs/fuse: handle idmappings properly in ->write_iter
  fs/fuse: warn if fuse_access is called when idmapped mounts are
    allowed
  fs/fuse: allow idmapped mounts
  fs/fuse/virtio_fs: allow idmapped mounts

 fs/fuse/acl.c             |  10 +--
 fs/fuse/dax.c             |   4 +-
 fs/fuse/dev.c             |  54 +++++++++---
 fs/fuse/dir.c             | 169 ++++++++++++++++++++++----------------
 fs/fuse/file.c            |  37 +++++----
 fs/fuse/fuse_i.h          |   7 +-
 fs/fuse/inode.c           |  19 +++--
 fs/fuse/ioctl.c           |   2 +-
 fs/fuse/readdir.c         |   4 +-
 fs/fuse/virtio_fs.c       |   1 +
 fs/fuse/xattr.c           |   8 +-
 fs/namespace.c            |   4 +
 include/linux/fs.h        |   1 +
 include/uapi/linux/fuse.h |  22 ++++-
 14 files changed, 216 insertions(+), 126 deletions(-)

-- 
2.34.1


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

* [PATCH v4 01/15] fs/namespace: introduce SB_I_NOIDMAP flag
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 02/15] fs/fuse: add basic infrastructure to support idmappings Alexander Mikhalitsyn
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	Alexander Viro, Jan Kara, linux-kernel

Right now we determine if filesystem support vfs idmappings
or not basing on the FS_ALLOW_IDMAP flag presence. This
"static" way works perfecly well for local filesystems
like ext4, xfs, btrfs, etc. But for network-like filesystems
like fuse, cephfs this approach is not ideal, because sometimes
proper support of vfs idmaps requires some extensions for the on-wire
protocol, which implies that changes have to be made not only
in the Linux kernel code but also in the 3rd party components like
libfuse, cephfs MDS server and so on.

We have seen that issue during our work on cephfs idmapped mounts [1]
with Christian, but right now I'm working on the idmapped mounts
support for fuse/virtiofs and I think that it is a right time for this extension.

[1] 5ccd8530dd7 ("ceph: handle idmapped mounts in create_request_message()")

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Suggested-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Reviewed-by: Christian Brauner <brauner@kernel.org>
---
v3:
	- this commit added
---
 fs/namespace.c     | 4 ++++
 include/linux/fs.h | 1 +
 2 files changed, 5 insertions(+)

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 6ca11e241a24..8756f84d627c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1190,6 +1190,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 {
-- 
2.34.1


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

* [PATCH v4 02/15] fs/fuse: add basic infrastructure to support idmappings
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 01/15] fs/namespace: introduce SB_I_NOIDMAP flag Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 03/15] fs/fuse: add an idmap argument to fuse_simple_request Alexander Mikhalitsyn
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	linux-kernel

Add some preparational changes in fuse_get_req/fuse_force_creds
to handle idmappings.

Miklos suggested [1], [2] to change the meaning of in.h.uid/in.h.gid
fields when daemon declares support for idmapped mounts. In a new semantic,
we fill uid/gid values in fuse header with a id-mapped caller uid/gid (for
requests which create new inodes), for all the rest cases we just send -1
to userspace.

No functional changes intended.

Link: https://lore.kernel.org/all/CAJfpegsVY97_5mHSc06mSw79FehFWtoXT=hhTUK_E-Yhr7OAuQ@mail.gmail.com/ [1]
Link: https://lore.kernel.org/all/CAJfpegtHQsEUuFq1k4ZbTD3E1h-GsrN3PWyv7X8cg6sfU_W2Yw@mail.gmail.com/ [2]
Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
---
v4:
        - this commit added
---
 fs/fuse/dev.c             | 50 +++++++++++++++++++++++++++++----------
 fs/fuse/inode.c           |  1 +
 include/uapi/linux/fuse.h |  2 ++
 3 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 7146038b2fe7..d3f3c3557c04 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -103,7 +103,9 @@ static void fuse_drop_waiting(struct fuse_conn *fc)
 
 static void fuse_put_request(struct fuse_req *req);
 
-static struct fuse_req *fuse_get_req(struct fuse_mount *fm, bool for_background)
+static struct fuse_req *fuse_get_req(struct mnt_idmap *idmap,
+				     struct fuse_mount *fm,
+				     bool for_background)
 {
 	struct fuse_conn *fc = fm->fc;
 	struct fuse_req *req;
@@ -135,19 +137,37 @@ static struct fuse_req *fuse_get_req(struct fuse_mount *fm, bool for_background)
 		goto out;
 	}
 
-	req->in.h.uid = from_kuid(fc->user_ns, current_fsuid());
-	req->in.h.gid = from_kgid(fc->user_ns, current_fsgid());
 	req->in.h.pid = pid_nr_ns(task_pid(current), fc->pid_ns);
 
 	__set_bit(FR_WAITING, &req->flags);
 	if (for_background)
 		__set_bit(FR_BACKGROUND, &req->flags);
 
-	if (unlikely(req->in.h.uid == ((uid_t)-1) ||
-		     req->in.h.gid == ((gid_t)-1))) {
-		fuse_put_request(req);
-		return ERR_PTR(-EOVERFLOW);
+	if ((fm->sb->s_iflags & SB_I_NOIDMAP) || idmap) {
+		kuid_t idmapped_fsuid;
+		kgid_t idmapped_fsgid;
+
+		/*
+		 * Note, that when
+		 * (fm->sb->s_iflags & SB_I_NOIDMAP) is true, then
+		 * (idmap == &nop_mnt_idmap) is always true and therefore,
+		 * mapped_fsuid(idmap, fc->user_ns) == current_fsuid().
+		 */
+		idmapped_fsuid = idmap ? mapped_fsuid(idmap, fc->user_ns) : current_fsuid();
+		idmapped_fsgid = idmap ? mapped_fsgid(idmap, fc->user_ns) : current_fsgid();
+		req->in.h.uid = from_kuid(fc->user_ns, idmapped_fsuid);
+		req->in.h.gid = from_kgid(fc->user_ns, idmapped_fsgid);
+
+		if (unlikely(req->in.h.uid == ((uid_t)-1) ||
+			     req->in.h.gid == ((gid_t)-1))) {
+			fuse_put_request(req);
+			return ERR_PTR(-EOVERFLOW);
+		}
+	} else {
+		req->in.h.uid = FUSE_INVALID_UIDGID;
+		req->in.h.gid = FUSE_INVALID_UIDGID;
 	}
+
 	return req;
 
  out:
@@ -466,8 +486,14 @@ static void fuse_force_creds(struct fuse_req *req)
 {
 	struct fuse_conn *fc = req->fm->fc;
 
-	req->in.h.uid = from_kuid_munged(fc->user_ns, current_fsuid());
-	req->in.h.gid = from_kgid_munged(fc->user_ns, current_fsgid());
+	if (req->fm->sb->s_iflags & SB_I_NOIDMAP) {
+		req->in.h.uid = from_kuid_munged(fc->user_ns, current_fsuid());
+		req->in.h.gid = from_kgid_munged(fc->user_ns, current_fsgid());
+	} else {
+		req->in.h.uid = FUSE_INVALID_UIDGID;
+		req->in.h.gid = FUSE_INVALID_UIDGID;
+	}
+
 	req->in.h.pid = pid_nr_ns(task_pid(current), fc->pid_ns);
 }
 
@@ -499,7 +525,7 @@ ssize_t fuse_simple_request(struct fuse_mount *fm, struct fuse_args *args)
 		__set_bit(FR_FORCE, &req->flags);
 	} else {
 		WARN_ON(args->nocreds);
-		req = fuse_get_req(fm, false);
+		req = fuse_get_req(NULL, fm, false);
 		if (IS_ERR(req))
 			return PTR_ERR(req);
 	}
@@ -560,7 +586,7 @@ int fuse_simple_background(struct fuse_mount *fm, struct fuse_args *args,
 		__set_bit(FR_BACKGROUND, &req->flags);
 	} else {
 		WARN_ON(args->nocreds);
-		req = fuse_get_req(fm, true);
+		req = fuse_get_req(NULL, fm, true);
 		if (IS_ERR(req))
 			return PTR_ERR(req);
 	}
@@ -583,7 +609,7 @@ static int fuse_simple_notify_reply(struct fuse_mount *fm,
 	struct fuse_iqueue *fiq = &fm->fc->iq;
 	int err = 0;
 
-	req = fuse_get_req(fm, false);
+	req = fuse_get_req(NULL, fm, false);
 	if (IS_ERR(req))
 		return PTR_ERR(req);
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index d8ab4e93916f..115538f6f283 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1567,6 +1567,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/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index d08b99d60f6f..2ccf38181df2 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -984,6 +984,8 @@ struct fuse_fallocate_in {
  */
 #define FUSE_UNIQUE_RESEND (1ULL << 63)
 
+#define FUSE_INVALID_UIDGID ((uint32_t)(-1))
+
 struct fuse_in_header {
 	uint32_t	len;
 	uint32_t	opcode;
-- 
2.34.1


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

* [PATCH v4 03/15] fs/fuse: add an idmap argument to fuse_simple_request
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 01/15] fs/namespace: introduce SB_I_NOIDMAP flag Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 02/15] fs/fuse: add basic infrastructure to support idmappings Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-04 15:25   ` Christian Brauner
  2024-09-03 15:16 ` [PATCH v4 04/15] fs/fuse: support idmapped FUSE_EXT_GROUPS Alexander Mikhalitsyn
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	linux-kernel

If idmap == NULL *and* filesystem daemon declared idmapped mounts
support, then uid/gid values in a fuse header will be -1.

No functional changes intended.

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
---
v4:
	- this commit added
---
 fs/fuse/dax.c     |  4 ++--
 fs/fuse/dev.c     |  6 ++++--
 fs/fuse/dir.c     | 26 +++++++++++++-------------
 fs/fuse/file.c    | 32 ++++++++++++++++----------------
 fs/fuse/fuse_i.h  |  3 ++-
 fs/fuse/inode.c   |  6 +++---
 fs/fuse/ioctl.c   |  2 +-
 fs/fuse/readdir.c |  4 ++--
 fs/fuse/xattr.c   |  8 ++++----
 9 files changed, 47 insertions(+), 44 deletions(-)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index 12ef91d170bb..6d8368d66dd4 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -207,7 +207,7 @@ static int fuse_setup_one_mapping(struct inode *inode, unsigned long start_idx,
 	args.in_numargs = 1;
 	args.in_args[0].size = sizeof(inarg);
 	args.in_args[0].value = &inarg;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (err < 0)
 		return err;
 	dmap->writable = writable;
@@ -245,7 +245,7 @@ static int fuse_send_removemapping(struct inode *inode,
 	args.in_args[0].value = inargp;
 	args.in_args[1].size = inargp->count * sizeof(*remove_one);
 	args.in_args[1].value = remove_one;
-	return fuse_simple_request(fm, &args);
+	return fuse_simple_request(NULL, fm, &args);
 }
 
 static int dmap_removemapping_list(struct inode *inode, unsigned int num,
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index d3f3c3557c04..349fc84897a5 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -508,7 +508,9 @@ static void fuse_args_to_req(struct fuse_req *req, struct fuse_args *args)
 		__set_bit(FR_ASYNC, &req->flags);
 }
 
-ssize_t fuse_simple_request(struct fuse_mount *fm, struct fuse_args *args)
+ssize_t fuse_simple_request(struct mnt_idmap *idmap,
+			    struct fuse_mount *fm,
+			    struct fuse_args *args)
 {
 	struct fuse_conn *fc = fm->fc;
 	struct fuse_req *req;
@@ -525,7 +527,7 @@ ssize_t fuse_simple_request(struct fuse_mount *fm, struct fuse_args *args)
 		__set_bit(FR_FORCE, &req->flags);
 	} else {
 		WARN_ON(args->nocreds);
-		req = fuse_get_req(NULL, fm, false);
+		req = fuse_get_req(idmap, fm, false);
 		if (IS_ERR(req))
 			return PTR_ERR(req);
 	}
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 2b0d4781f394..2a8344776350 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -230,7 +230,7 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
 		parent = dget_parent(entry);
 		fuse_lookup_init(fm->fc, &args, get_node_id(d_inode(parent)),
 				 &entry->d_name, &outarg);
-		ret = fuse_simple_request(fm, &args);
+		ret = fuse_simple_request(NULL, fm, &args);
 		dput(parent);
 		/* Zero nodeid is same as -ENOENT */
 		if (!ret && !outarg.nodeid)
@@ -383,7 +383,7 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
 	attr_version = fuse_get_attr_version(fm->fc);
 
 	fuse_lookup_init(fm->fc, &args, nodeid, name, outarg);
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	/* Zero nodeid is same as -ENOENT, but with valid timeout */
 	if (err || !outarg->nodeid)
 		goto out_put_forget;
@@ -672,7 +672,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	if (err)
 		goto out_put_forget_req;
 
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	free_ext_value(&args);
 	if (err)
 		goto out_free_ff;
@@ -803,7 +803,7 @@ static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
 			goto out_put_forget_req;
 	}
 
-	err = fuse_simple_request(fm, args);
+	err = fuse_simple_request(NULL, fm, args);
 	free_ext_value(args);
 	if (err)
 		goto out_put_forget_req;
@@ -987,7 +987,7 @@ static int fuse_unlink(struct inode *dir, struct dentry *entry)
 	args.in_numargs = 1;
 	args.in_args[0].size = entry->d_name.len + 1;
 	args.in_args[0].value = entry->d_name.name;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (!err) {
 		fuse_dir_changed(dir);
 		fuse_entry_unlinked(entry);
@@ -1010,7 +1010,7 @@ static int fuse_rmdir(struct inode *dir, struct dentry *entry)
 	args.in_numargs = 1;
 	args.in_args[0].size = entry->d_name.len + 1;
 	args.in_args[0].value = entry->d_name.name;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (!err) {
 		fuse_dir_changed(dir);
 		fuse_entry_unlinked(entry);
@@ -1040,7 +1040,7 @@ static int fuse_rename_common(struct inode *olddir, struct dentry *oldent,
 	args.in_args[1].value = oldent->d_name.name;
 	args.in_args[2].size = newent->d_name.len + 1;
 	args.in_args[2].value = newent->d_name.name;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (!err) {
 		/* ctime changes */
 		fuse_update_ctime(d_inode(oldent));
@@ -1210,7 +1210,7 @@ static int fuse_do_statx(struct inode *inode, struct file *file,
 	args.out_numargs = 1;
 	args.out_args[0].size = sizeof(outarg);
 	args.out_args[0].value = &outarg;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (err)
 		return err;
 
@@ -1268,7 +1268,7 @@ static int fuse_do_getattr(struct inode *inode, struct kstat *stat,
 	args.out_numargs = 1;
 	args.out_args[0].size = sizeof(outarg);
 	args.out_args[0].value = &outarg;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (!err) {
 		if (fuse_invalid_attr(&outarg.attr) ||
 		    inode_wrong_type(inode, outarg.attr.mode)) {
@@ -1472,7 +1472,7 @@ static int fuse_access(struct inode *inode, int mask)
 	args.in_numargs = 1;
 	args.in_args[0].size = sizeof(inarg);
 	args.in_args[0].value = &inarg;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (err == -ENOSYS) {
 		fm->fc->no_access = 1;
 		err = 0;
@@ -1584,7 +1584,7 @@ static int fuse_readlink_page(struct inode *inode, struct page *page)
 	ap.args.page_zeroing = true;
 	ap.args.out_numargs = 1;
 	ap.args.out_args[0].size = desc.length;
-	res = fuse_simple_request(fm, &ap.args);
+	res = fuse_simple_request(NULL, fm, &ap.args);
 
 	fuse_invalidate_atime(inode);
 
@@ -1857,7 +1857,7 @@ int fuse_flush_times(struct inode *inode, struct fuse_file *ff)
 	}
 	fuse_setattr_fill(fm->fc, &args, inode, &inarg, &outarg);
 
-	return fuse_simple_request(fm, &args);
+	return fuse_simple_request(NULL, fm, &args);
 }
 
 /*
@@ -1970,7 +1970,7 @@ int fuse_do_setattr(struct dentry *dentry, struct iattr *attr,
 			inarg.valid |= FATTR_KILL_SUIDGID;
 	}
 	fuse_setattr_fill(fc, &args, inode, &inarg, &outarg);
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (err) {
 		if (err == -EINTR)
 			fuse_invalidate_attr(inode);
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index f39456c65ed7..7d14d533dad1 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -48,7 +48,7 @@ static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
 	args.out_args[0].size = sizeof(*outargp);
 	args.out_args[0].value = outargp;
 
-	return fuse_simple_request(fm, &args);
+	return fuse_simple_request(NULL, fm, &args);
 }
 
 struct fuse_file *fuse_file_alloc(struct fuse_mount *fm, bool release)
@@ -111,7 +111,7 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
 		if (!args) {
 			/* Do nothing when server does not implement 'open' */
 		} else if (sync) {
-			fuse_simple_request(ff->fm, args);
+			fuse_simple_request(NULL, ff->fm, args);
 			fuse_release_end(ff->fm, args, 0);
 		} else {
 			args->end = fuse_release_end;
@@ -539,7 +539,7 @@ static int fuse_flush(struct file *file, fl_owner_t id)
 	args.in_args[0].value = &inarg;
 	args.force = true;
 
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (err == -ENOSYS) {
 		fm->fc->no_flush = 1;
 		err = 0;
@@ -572,7 +572,7 @@ int fuse_fsync_common(struct file *file, loff_t start, loff_t end,
 	args.in_numargs = 1;
 	args.in_args[0].size = sizeof(inarg);
 	args.in_args[0].value = &inarg;
-	return fuse_simple_request(fm, &args);
+	return fuse_simple_request(NULL, fm, &args);
 }
 
 static int fuse_fsync(struct file *file, loff_t start, loff_t end,
@@ -814,7 +814,7 @@ static ssize_t fuse_send_read(struct fuse_io_args *ia, loff_t pos, size_t count,
 	if (ia->io->async)
 		return fuse_async_req_send(fm, ia, count);
 
-	return fuse_simple_request(fm, &ia->ap.args);
+	return fuse_simple_request(NULL, fm, &ia->ap.args);
 }
 
 static void fuse_read_update_size(struct inode *inode, loff_t size,
@@ -878,7 +878,7 @@ static int fuse_do_readpage(struct file *file, struct page *page)
 		desc.length--;
 
 	fuse_read_args_fill(&ia, file, pos, desc.length, FUSE_READ);
-	res = fuse_simple_request(fm, &ia.ap.args);
+	res = fuse_simple_request(NULL, fm, &ia.ap.args);
 	if (res < 0)
 		return res;
 	/*
@@ -976,7 +976,7 @@ static void fuse_send_readpages(struct fuse_io_args *ia, struct file *file)
 		if (!err)
 			return;
 	} else {
-		res = fuse_simple_request(fm, &ap->args);
+		res = fuse_simple_request(NULL, fm, &ap->args);
 		err = res < 0 ? res : 0;
 	}
 	fuse_readpages_end(fm, &ap->args, err);
@@ -1101,7 +1101,7 @@ static ssize_t fuse_send_write(struct fuse_io_args *ia, loff_t pos,
 	if (ia->io->async)
 		return fuse_async_req_send(fm, ia, count);
 
-	err = fuse_simple_request(fm, &ia->ap.args);
+	err = fuse_simple_request(NULL, fm, &ia->ap.args);
 	if (!err && ia->write.out.size > count)
 		err = -EIO;
 
@@ -1147,7 +1147,7 @@ static ssize_t fuse_send_write_pages(struct fuse_io_args *ia,
 	if (fm->fc->handle_killpriv_v2 && !capable(CAP_FSETID))
 		ia->write.in.write_flags |= FUSE_WRITE_KILL_SUIDGID;
 
-	err = fuse_simple_request(fm, &ap->args);
+	err = fuse_simple_request(NULL, fm, &ap->args);
 	if (!err && ia->write.out.size > count)
 		err = -EIO;
 
@@ -2656,7 +2656,7 @@ static int fuse_getlk(struct file *file, struct file_lock *fl)
 	args.out_numargs = 1;
 	args.out_args[0].size = sizeof(outarg);
 	args.out_args[0].value = &outarg;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (!err)
 		err = convert_fuse_file_lock(fm->fc, &outarg.lk, fl);
 
@@ -2680,7 +2680,7 @@ static int fuse_setlk(struct file *file, struct file_lock *fl, int flock)
 	}
 
 	fuse_lk_fill(&args, file, fl, opcode, pid_nr, flock, &inarg);
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 
 	/* locking is restartable */
 	if (err == -EINTR)
@@ -2754,7 +2754,7 @@ static sector_t fuse_bmap(struct address_space *mapping, sector_t block)
 	args.out_numargs = 1;
 	args.out_args[0].size = sizeof(outarg);
 	args.out_args[0].value = &outarg;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (err == -ENOSYS)
 		fm->fc->no_bmap = 1;
 
@@ -2786,7 +2786,7 @@ static loff_t fuse_lseek(struct file *file, loff_t offset, int whence)
 	args.out_numargs = 1;
 	args.out_args[0].size = sizeof(outarg);
 	args.out_args[0].value = &outarg;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (err) {
 		if (err == -ENOSYS) {
 			fm->fc->no_lseek = 1;
@@ -2919,7 +2919,7 @@ __poll_t fuse_file_poll(struct file *file, poll_table *wait)
 	args.out_numargs = 1;
 	args.out_args[0].size = sizeof(outarg);
 	args.out_args[0].value = &outarg;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 
 	if (!err)
 		return demangle_poll(outarg.revents);
@@ -3141,7 +3141,7 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
 	args.in_numargs = 1;
 	args.in_args[0].size = sizeof(inarg);
 	args.in_args[0].value = &inarg;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (err == -ENOSYS) {
 		fm->fc->no_fallocate = 1;
 		err = -EOPNOTSUPP;
@@ -3253,7 +3253,7 @@ static ssize_t __fuse_copy_file_range(struct file *file_in, loff_t pos_in,
 	args.out_numargs = 1;
 	args.out_args[0].size = sizeof(outarg);
 	args.out_args[0].value = &outarg;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (err == -ENOSYS) {
 		fc->no_copy_file_range = 1;
 		err = -EOPNOTSUPP;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index f23919610313..656575e3e4cf 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1154,7 +1154,8 @@ void __exit fuse_ctl_cleanup(void);
 /**
  * Simple request sending that does request allocation and freeing
  */
-ssize_t fuse_simple_request(struct fuse_mount *fm, struct fuse_args *args);
+ssize_t fuse_simple_request(struct mnt_idmap *idmap, struct fuse_mount *fm,
+			    struct fuse_args *args);
 int fuse_simple_background(struct fuse_mount *fm, struct fuse_args *args,
 			   gfp_t gfp_flags);
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 115538f6f283..2e26810066e8 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -586,7 +586,7 @@ static void fuse_send_destroy(struct fuse_mount *fm)
 		args.opcode = FUSE_DESTROY;
 		args.force = true;
 		args.nocreds = true;
-		fuse_simple_request(fm, &args);
+		fuse_simple_request(NULL, fm, &args);
 	}
 }
 
@@ -624,7 +624,7 @@ static int fuse_statfs(struct dentry *dentry, struct kstatfs *buf)
 	args.out_numargs = 1;
 	args.out_args[0].size = sizeof(outarg);
 	args.out_args[0].value = &outarg;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (!err)
 		convert_fuse_statfs(buf, &outarg.st);
 	return err;
@@ -713,7 +713,7 @@ static int fuse_sync_fs(struct super_block *sb, int wait)
 	args.nodeid = get_node_id(sb->s_root->d_inode);
 	args.out_numargs = 0;
 
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (err == -ENOSYS) {
 		fc->sync_fs = 0;
 		err = 0;
diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
index 572ce8a82ceb..b40dd931167d 100644
--- a/fs/fuse/ioctl.c
+++ b/fs/fuse/ioctl.c
@@ -18,7 +18,7 @@ static ssize_t fuse_send_ioctl(struct fuse_mount *fm, struct fuse_args *args,
 	args->out_args[0].size = sizeof(*outarg);
 	args->out_args[0].value = outarg;
 
-	ret = fuse_simple_request(fm, args);
+	ret = fuse_simple_request(NULL, fm, args);
 
 	/* Translate ENOSYS, which shouldn't be returned from fs */
 	if (ret == -ENOSYS)
diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
index 0377b6dc24c8..e8a093289421 100644
--- a/fs/fuse/readdir.c
+++ b/fs/fuse/readdir.c
@@ -279,7 +279,7 @@ static void fuse_force_forget(struct file *file, u64 nodeid)
 	args.force = true;
 	args.noreply = true;
 
-	fuse_simple_request(fm, &args);
+	fuse_simple_request(NULL, fm, &args);
 	/* ignore errors */
 }
 
@@ -358,7 +358,7 @@ static int fuse_readdir_uncached(struct file *file, struct dir_context *ctx)
 				    FUSE_READDIR);
 	}
 	locked = fuse_lock_inode(inode);
-	res = fuse_simple_request(fm, &ap->args);
+	res = fuse_simple_request(NULL, fm, &ap->args);
 	fuse_unlock_inode(inode, locked);
 	if (res >= 0) {
 		if (!res) {
diff --git a/fs/fuse/xattr.c b/fs/fuse/xattr.c
index 5b423fdbb13f..6f8f1453b550 100644
--- a/fs/fuse/xattr.c
+++ b/fs/fuse/xattr.c
@@ -37,7 +37,7 @@ int fuse_setxattr(struct inode *inode, const char *name, const void *value,
 	args.in_args[1].value = name;
 	args.in_args[2].size = size;
 	args.in_args[2].value = value;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (err == -ENOSYS) {
 		fm->fc->no_setxattr = 1;
 		err = -EOPNOTSUPP;
@@ -79,7 +79,7 @@ ssize_t fuse_getxattr(struct inode *inode, const char *name, void *value,
 		args.out_args[0].size = sizeof(outarg);
 		args.out_args[0].value = &outarg;
 	}
-	ret = fuse_simple_request(fm, &args);
+	ret = fuse_simple_request(NULL, fm, &args);
 	if (!ret && !size)
 		ret = min_t(ssize_t, outarg.size, XATTR_SIZE_MAX);
 	if (ret == -ENOSYS) {
@@ -141,7 +141,7 @@ ssize_t fuse_listxattr(struct dentry *entry, char *list, size_t size)
 		args.out_args[0].size = sizeof(outarg);
 		args.out_args[0].value = &outarg;
 	}
-	ret = fuse_simple_request(fm, &args);
+	ret = fuse_simple_request(NULL, fm, &args);
 	if (!ret && !size)
 		ret = min_t(ssize_t, outarg.size, XATTR_LIST_MAX);
 	if (ret > 0 && size)
@@ -167,7 +167,7 @@ int fuse_removexattr(struct inode *inode, const char *name)
 	args.in_numargs = 1;
 	args.in_args[0].size = strlen(name) + 1;
 	args.in_args[0].value = name;
-	err = fuse_simple_request(fm, &args);
+	err = fuse_simple_request(NULL, fm, &args);
 	if (err == -ENOSYS) {
 		fm->fc->no_removexattr = 1;
 		err = -EOPNOTSUPP;
-- 
2.34.1


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

* [PATCH v4 04/15] fs/fuse: support idmapped FUSE_EXT_GROUPS
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
                   ` (2 preceding siblings ...)
  2024-09-03 15:16 ` [PATCH v4 03/15] fs/fuse: add an idmap argument to fuse_simple_request Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 05/15] fs/fuse: support idmap for mkdir/mknod/symlink/create/tmpfile Alexander Mikhalitsyn
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	linux-kernel

We don't need to remap parent_gid, but have to adjust
group membership checks and take idmapping into account.

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
---
v4:
	- this commit added
---
 fs/fuse/dir.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 2a8344776350..b0b57f383889 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -545,17 +545,21 @@ static u32 fuse_ext_size(size_t size)
 /*
  * This adds just a single supplementary group that matches the parent's group.
  */
-static int get_create_supp_group(struct inode *dir, struct fuse_in_arg *ext)
+static int get_create_supp_group(struct mnt_idmap *idmap,
+				 struct inode *dir,
+				 struct fuse_in_arg *ext)
 {
 	struct fuse_conn *fc = get_fuse_conn(dir);
 	struct fuse_ext_header *xh;
 	struct fuse_supp_groups *sg;
 	kgid_t kgid = dir->i_gid;
+	vfsgid_t vfsgid = make_vfsgid(idmap, fc->user_ns, kgid);
 	gid_t parent_gid = from_kgid(fc->user_ns, kgid);
+
 	u32 sg_len = fuse_ext_size(sizeof(*sg) + sizeof(sg->groups[0]));
 
-	if (parent_gid == (gid_t) -1 || gid_eq(kgid, current_fsgid()) ||
-	    !in_group_p(kgid))
+	if (parent_gid == (gid_t) -1 || vfsgid_eq_kgid(vfsgid, current_fsgid()) ||
+	    !vfsgid_in_group_p(vfsgid))
 		return 0;
 
 	xh = extend_arg(ext, sg_len);
@@ -572,7 +576,8 @@ static int get_create_supp_group(struct inode *dir, struct fuse_in_arg *ext)
 	return 0;
 }
 
-static int get_create_ext(struct fuse_args *args,
+static int get_create_ext(struct mnt_idmap *idmap,
+			  struct fuse_args *args,
 			  struct inode *dir, struct dentry *dentry,
 			  umode_t mode)
 {
@@ -583,7 +588,7 @@ static int get_create_ext(struct fuse_args *args,
 	if (fc->init_security)
 		err = get_security_context(dentry, mode, &ext);
 	if (!err && fc->create_supp_group)
-		err = get_create_supp_group(dir, &ext);
+		err = get_create_supp_group(idmap, dir, &ext);
 
 	if (!err && ext.size) {
 		WARN_ON(args->in_numargs >= ARRAY_SIZE(args->in_args));
@@ -668,7 +673,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	args.out_args[1].size = sizeof(*outopenp);
 	args.out_args[1].value = outopenp;
 
-	err = get_create_ext(&args, dir, entry, mode);
+	err = get_create_ext(&nop_mnt_idmap, &args, dir, entry, mode);
 	if (err)
 		goto out_put_forget_req;
 
@@ -798,7 +803,7 @@ static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
 	args->out_args[0].value = &outarg;
 
 	if (args->opcode != FUSE_LINK) {
-		err = get_create_ext(args, dir, entry, mode);
+		err = get_create_ext(&nop_mnt_idmap, args, dir, entry, mode);
 		if (err)
 			goto out_put_forget_req;
 	}
-- 
2.34.1


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

* [PATCH v4 05/15] fs/fuse: support idmap for mkdir/mknod/symlink/create/tmpfile
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
                   ` (3 preceding siblings ...)
  2024-09-03 15:16 ` [PATCH v4 04/15] fs/fuse: support idmapped FUSE_EXT_GROUPS Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 06/15] fs/fuse: support idmapped getattr inode op Alexander Mikhalitsyn
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	linux-kernel

We have all the infrastructure in place, we just need
to pass an idmapping here.

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Reviewed-by: Christian Brauner <brauner@kernel.org>
---
v4:
	- pass idmapping to fuse_simple_request()
---
 fs/fuse/dir.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index b0b57f383889..19538b1c12e2 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -614,9 +614,9 @@ static void free_ext_value(struct fuse_args *args)
  * If the filesystem doesn't support this, then fall back to separate
  * 'mknod' + 'open' requests.
  */
-static int fuse_create_open(struct inode *dir, struct dentry *entry,
-			    struct file *file, unsigned int flags,
-			    umode_t mode, u32 opcode)
+static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
+			    struct dentry *entry, struct file *file,
+			    unsigned int flags, umode_t mode, u32 opcode)
 {
 	int err;
 	struct inode *inode;
@@ -673,11 +673,11 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	args.out_args[1].size = sizeof(*outopenp);
 	args.out_args[1].value = outopenp;
 
-	err = get_create_ext(&nop_mnt_idmap, &args, dir, entry, mode);
+	err = get_create_ext(idmap, &args, dir, entry, mode);
 	if (err)
 		goto out_put_forget_req;
 
-	err = fuse_simple_request(NULL, fm, &args);
+	err = fuse_simple_request(idmap, fm, &args);
 	free_ext_value(&args);
 	if (err)
 		goto out_free_ff;
@@ -734,6 +734,7 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
 			    umode_t mode)
 {
 	int err;
+	struct mnt_idmap *idmap = file_mnt_idmap(file);
 	struct fuse_conn *fc = get_fuse_conn(dir);
 	struct dentry *res = NULL;
 
@@ -758,7 +759,7 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
 	if (fc->no_create)
 		goto mknod;
 
-	err = fuse_create_open(dir, entry, file, flags, mode, FUSE_CREATE);
+	err = fuse_create_open(idmap, dir, entry, file, flags, mode, FUSE_CREATE);
 	if (err == -ENOSYS) {
 		fc->no_create = 1;
 		goto mknod;
@@ -769,7 +770,7 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
 	return err;
 
 mknod:
-	err = fuse_mknod(&nop_mnt_idmap, dir, entry, mode, 0);
+	err = fuse_mknod(idmap, dir, entry, mode, 0);
 	if (err)
 		goto out_dput;
 no_open:
@@ -779,9 +780,9 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
 /*
  * Code shared between mknod, mkdir, symlink and link
  */
-static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
-			    struct inode *dir, struct dentry *entry,
-			    umode_t mode)
+static int create_new_entry(struct mnt_idmap *idmap, struct fuse_mount *fm,
+			    struct fuse_args *args, struct inode *dir,
+			    struct dentry *entry, umode_t mode)
 {
 	struct fuse_entry_out outarg;
 	struct inode *inode;
@@ -803,12 +804,12 @@ static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
 	args->out_args[0].value = &outarg;
 
 	if (args->opcode != FUSE_LINK) {
-		err = get_create_ext(&nop_mnt_idmap, args, dir, entry, mode);
+		err = get_create_ext(idmap, args, dir, entry, mode);
 		if (err)
 			goto out_put_forget_req;
 	}
 
-	err = fuse_simple_request(NULL, fm, args);
+	err = fuse_simple_request(idmap, fm, args);
 	free_ext_value(args);
 	if (err)
 		goto out_put_forget_req;
@@ -869,13 +870,13 @@ static int fuse_mknod(struct mnt_idmap *idmap, struct inode *dir,
 	args.in_args[0].value = &inarg;
 	args.in_args[1].size = entry->d_name.len + 1;
 	args.in_args[1].value = entry->d_name.name;
-	return create_new_entry(fm, &args, dir, entry, mode);
+	return create_new_entry(idmap, fm, &args, dir, entry, mode);
 }
 
 static int fuse_create(struct mnt_idmap *idmap, struct inode *dir,
 		       struct dentry *entry, umode_t mode, bool excl)
 {
-	return fuse_mknod(&nop_mnt_idmap, dir, entry, mode, 0);
+	return fuse_mknod(idmap, dir, entry, mode, 0);
 }
 
 static int fuse_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
@@ -887,7 +888,7 @@ static int fuse_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
 	if (fc->no_tmpfile)
 		return -EOPNOTSUPP;
 
-	err = fuse_create_open(dir, file->f_path.dentry, file, file->f_flags, mode, FUSE_TMPFILE);
+	err = fuse_create_open(idmap, dir, file->f_path.dentry, file, file->f_flags, mode, FUSE_TMPFILE);
 	if (err == -ENOSYS) {
 		fc->no_tmpfile = 1;
 		err = -EOPNOTSUPP;
@@ -914,7 +915,7 @@ static int fuse_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 	args.in_args[0].value = &inarg;
 	args.in_args[1].size = entry->d_name.len + 1;
 	args.in_args[1].value = entry->d_name.name;
-	return create_new_entry(fm, &args, dir, entry, S_IFDIR);
+	return create_new_entry(idmap, fm, &args, dir, entry, S_IFDIR);
 }
 
 static int fuse_symlink(struct mnt_idmap *idmap, struct inode *dir,
@@ -930,7 +931,7 @@ static int fuse_symlink(struct mnt_idmap *idmap, struct inode *dir,
 	args.in_args[0].value = entry->d_name.name;
 	args.in_args[1].size = len;
 	args.in_args[1].value = link;
-	return create_new_entry(fm, &args, dir, entry, S_IFLNK);
+	return create_new_entry(idmap, fm, &args, dir, entry, S_IFLNK);
 }
 
 void fuse_flush_time_update(struct inode *inode)
@@ -1124,7 +1125,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
 	args.in_args[0].value = &inarg;
 	args.in_args[1].size = newent->d_name.len + 1;
 	args.in_args[1].value = newent->d_name.name;
-	err = create_new_entry(fm, &args, newdir, newent, inode->i_mode);
+	err = create_new_entry(NULL, fm, &args, newdir, newent, inode->i_mode);
 	if (!err)
 		fuse_update_ctime_in_cache(inode);
 	else if (err == -EINTR)
-- 
2.34.1


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

* [PATCH v4 06/15] fs/fuse: support idmapped getattr inode op
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
                   ` (4 preceding siblings ...)
  2024-09-03 15:16 ` [PATCH v4 05/15] fs/fuse: support idmap for mkdir/mknod/symlink/create/tmpfile Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 07/15] fs/fuse: support idmapped ->permission " Alexander Mikhalitsyn
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	linux-kernel

We have to:
- pass an idmapping to the generic_fillattr()
to properly handle UIG/GID mapping for the userspace.
- pass -/- to fuse_fillattr() (analog of generic_fillattr() in fuse).

Difference between these two is that generic_fillattr() takes all
the stat() data from the inode directly, while fuse_fillattr() codepath
takes a fresh data just from the userspace reply on the FUSE_GETATTR request.

In some cases we can just pass &nop_mnt_idmap, because idmapping won't
be used in these codepaths. For example, when 3rd argument of fuse_do_getattr()
is NULL then idmap argument is not used.

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Reviewed-by: Christian Brauner <brauner@kernel.org>
---
v2:
	- pass idmap in more cases to make code easier to understand
---
 fs/fuse/dir.c | 44 ++++++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 19538b1c12e2..1c28cdf9dd41 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1134,18 +1134,22 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
 	return err;
 }
 
-static void fuse_fillattr(struct inode *inode, struct fuse_attr *attr,
-			  struct kstat *stat)
+static void fuse_fillattr(struct mnt_idmap *idmap, struct inode *inode,
+			  struct fuse_attr *attr, struct kstat *stat)
 {
 	unsigned int blkbits;
 	struct fuse_conn *fc = get_fuse_conn(inode);
+	vfsuid_t vfsuid = make_vfsuid(idmap, fc->user_ns,
+				      make_kuid(fc->user_ns, attr->uid));
+	vfsgid_t vfsgid = make_vfsgid(idmap, fc->user_ns,
+				      make_kgid(fc->user_ns, attr->gid));
 
 	stat->dev = inode->i_sb->s_dev;
 	stat->ino = attr->ino;
 	stat->mode = (inode->i_mode & S_IFMT) | (attr->mode & 07777);
 	stat->nlink = attr->nlink;
-	stat->uid = make_kuid(fc->user_ns, attr->uid);
-	stat->gid = make_kgid(fc->user_ns, attr->gid);
+	stat->uid = vfsuid_into_kuid(vfsuid);
+	stat->gid = vfsgid_into_kgid(vfsgid);
 	stat->rdev = inode->i_rdev;
 	stat->atime.tv_sec = attr->atime;
 	stat->atime.tv_nsec = attr->atimensec;
@@ -1184,8 +1188,8 @@ static void fuse_statx_to_attr(struct fuse_statx *sx, struct fuse_attr *attr)
 	attr->blksize = sx->blksize;
 }
 
-static int fuse_do_statx(struct inode *inode, struct file *file,
-			 struct kstat *stat)
+static int fuse_do_statx(struct mnt_idmap *idmap, struct inode *inode,
+			 struct file *file, struct kstat *stat)
 {
 	int err;
 	struct fuse_attr attr;
@@ -1238,15 +1242,15 @@ static int fuse_do_statx(struct inode *inode, struct file *file,
 		stat->result_mask = sx->mask & (STATX_BASIC_STATS | STATX_BTIME);
 		stat->btime.tv_sec = sx->btime.tv_sec;
 		stat->btime.tv_nsec = min_t(u32, sx->btime.tv_nsec, NSEC_PER_SEC - 1);
-		fuse_fillattr(inode, &attr, stat);
+		fuse_fillattr(idmap, inode, &attr, stat);
 		stat->result_mask |= STATX_TYPE;
 	}
 
 	return 0;
 }
 
-static int fuse_do_getattr(struct inode *inode, struct kstat *stat,
-			   struct file *file)
+static int fuse_do_getattr(struct mnt_idmap *idmap, struct inode *inode,
+			   struct kstat *stat, struct file *file)
 {
 	int err;
 	struct fuse_getattr_in inarg;
@@ -1285,15 +1289,15 @@ static int fuse_do_getattr(struct inode *inode, struct kstat *stat,
 					       ATTR_TIMEOUT(&outarg),
 					       attr_version);
 			if (stat)
-				fuse_fillattr(inode, &outarg.attr, stat);
+				fuse_fillattr(idmap, inode, &outarg.attr, stat);
 		}
 	}
 	return err;
 }
 
-static int fuse_update_get_attr(struct inode *inode, struct file *file,
-				struct kstat *stat, u32 request_mask,
-				unsigned int flags)
+static int fuse_update_get_attr(struct mnt_idmap *idmap, struct inode *inode,
+				struct file *file, struct kstat *stat,
+				u32 request_mask, unsigned int flags)
 {
 	struct fuse_inode *fi = get_fuse_inode(inode);
 	struct fuse_conn *fc = get_fuse_conn(inode);
@@ -1324,17 +1328,17 @@ static int fuse_update_get_attr(struct inode *inode, struct file *file,
 		forget_all_cached_acls(inode);
 		/* Try statx if BTIME is requested */
 		if (!fc->no_statx && (request_mask & ~STATX_BASIC_STATS)) {
-			err = fuse_do_statx(inode, file, stat);
+			err = fuse_do_statx(idmap, inode, file, stat);
 			if (err == -ENOSYS) {
 				fc->no_statx = 1;
 				err = 0;
 				goto retry;
 			}
 		} else {
-			err = fuse_do_getattr(inode, stat, file);
+			err = fuse_do_getattr(idmap, inode, stat, file);
 		}
 	} else if (stat) {
-		generic_fillattr(&nop_mnt_idmap, request_mask, inode, stat);
+		generic_fillattr(idmap, request_mask, inode, stat);
 		stat->mode = fi->orig_i_mode;
 		stat->ino = fi->orig_ino;
 		if (test_bit(FUSE_I_BTIME, &fi->state)) {
@@ -1348,7 +1352,7 @@ static int fuse_update_get_attr(struct inode *inode, struct file *file,
 
 int fuse_update_attributes(struct inode *inode, struct file *file, u32 mask)
 {
-	return fuse_update_get_attr(inode, file, NULL, mask, 0);
+	return fuse_update_get_attr(&nop_mnt_idmap, inode, file, NULL, mask, 0);
 }
 
 int fuse_reverse_inval_entry(struct fuse_conn *fc, u64 parent_nodeid,
@@ -1492,7 +1496,7 @@ static int fuse_perm_getattr(struct inode *inode, int mask)
 		return -ECHILD;
 
 	forget_all_cached_acls(inode);
-	return fuse_do_getattr(inode, NULL, NULL);
+	return fuse_do_getattr(&nop_mnt_idmap, inode, NULL, NULL);
 }
 
 /*
@@ -2071,7 +2075,7 @@ static int fuse_setattr(struct mnt_idmap *idmap, struct dentry *entry,
 			 * ia_mode calculation may have used stale i_mode.
 			 * Refresh and recalculate.
 			 */
-			ret = fuse_do_getattr(inode, NULL, file);
+			ret = fuse_do_getattr(idmap, inode, NULL, file);
 			if (ret)
 				return ret;
 
@@ -2128,7 +2132,7 @@ static int fuse_getattr(struct mnt_idmap *idmap,
 		return -EACCES;
 	}
 
-	return fuse_update_get_attr(inode, NULL, stat, request_mask, flags);
+	return fuse_update_get_attr(idmap, inode, NULL, stat, request_mask, flags);
 }
 
 static const struct inode_operations fuse_dir_inode_operations = {
-- 
2.34.1


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

* [PATCH v4 07/15] fs/fuse: support idmapped ->permission inode op
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
                   ` (5 preceding siblings ...)
  2024-09-03 15:16 ` [PATCH v4 06/15] fs/fuse: support idmapped getattr inode op Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 08/15] fs/fuse: support idmapped ->setattr op Alexander Mikhalitsyn
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	linux-kernel

We only cover the case when "default_permissions" flag
is used. A reason for that is that otherwise all the permission
checks are done in the userspace and we have to deal with
VFS idmapping in the userspace (which is bad), alternatively
we have to provide the userspace with idmapped req->in.h.uid/req->in.h.gid
which is also not align with VFS idmaps philosophy.

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Reviewed-by: Christian Brauner <brauner@kernel.org>
---
 fs/fuse/dir.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 1c28cdf9dd41..870932543aa0 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1544,7 +1544,7 @@ static int fuse_permission(struct mnt_idmap *idmap,
 	}
 
 	if (fc->default_permissions) {
-		err = generic_permission(&nop_mnt_idmap, inode, mask);
+		err = generic_permission(idmap, inode, mask);
 
 		/* If permission is denied, try to refresh file
 		   attributes.  This is also needed, because the root
@@ -1552,7 +1552,7 @@ static int fuse_permission(struct mnt_idmap *idmap,
 		if (err == -EACCES && !refreshed) {
 			err = fuse_perm_getattr(inode, mask);
 			if (!err)
-				err = generic_permission(&nop_mnt_idmap,
+				err = generic_permission(idmap,
 							 inode, mask);
 		}
 
-- 
2.34.1


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

* [PATCH v4 08/15] fs/fuse: support idmapped ->setattr op
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
                   ` (6 preceding siblings ...)
  2024-09-03 15:16 ` [PATCH v4 07/15] fs/fuse: support idmapped ->permission " Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 09/15] fs/fuse: drop idmap argument from __fuse_get_acl Alexander Mikhalitsyn
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	linux-kernel

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Reviewed-by: Christian Brauner <brauner@kernel.org>
---
v2:
	- pass idmap in more cases to make code easier to understand
---
 fs/fuse/dir.c    | 32 +++++++++++++++++++++-----------
 fs/fuse/file.c   |  2 +-
 fs/fuse/fuse_i.h |  4 ++--
 3 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 870932543aa0..08bf9cc51a65 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1748,17 +1748,27 @@ static bool update_mtime(unsigned ivalid, bool trust_local_mtime)
 	return true;
 }
 
-static void iattr_to_fattr(struct fuse_conn *fc, struct iattr *iattr,
-			   struct fuse_setattr_in *arg, bool trust_local_cmtime)
+static void iattr_to_fattr(struct mnt_idmap *idmap, struct fuse_conn *fc,
+			   struct iattr *iattr, struct fuse_setattr_in *arg,
+			   bool trust_local_cmtime)
 {
 	unsigned ivalid = iattr->ia_valid;
 
 	if (ivalid & ATTR_MODE)
 		arg->valid |= FATTR_MODE,   arg->mode = iattr->ia_mode;
-	if (ivalid & ATTR_UID)
-		arg->valid |= FATTR_UID,    arg->uid = from_kuid(fc->user_ns, iattr->ia_uid);
-	if (ivalid & ATTR_GID)
-		arg->valid |= FATTR_GID,    arg->gid = from_kgid(fc->user_ns, iattr->ia_gid);
+
+	if (ivalid & ATTR_UID) {
+		kuid_t fsuid = from_vfsuid(idmap, fc->user_ns, iattr->ia_vfsuid);
+		arg->valid |= FATTR_UID;
+		arg->uid = from_kuid(fc->user_ns, fsuid);
+	}
+
+	if (ivalid & ATTR_GID) {
+		kgid_t fsgid = from_vfsgid(idmap, fc->user_ns, iattr->ia_vfsgid);
+		arg->valid |= FATTR_GID;
+		arg->gid = from_kgid(fc->user_ns, fsgid);
+	}
+
 	if (ivalid & ATTR_SIZE)
 		arg->valid |= FATTR_SIZE,   arg->size = iattr->ia_size;
 	if (ivalid & ATTR_ATIME) {
@@ -1878,8 +1888,8 @@ int fuse_flush_times(struct inode *inode, struct fuse_file *ff)
  * vmtruncate() doesn't allow for this case, so do the rlimit checking
  * and the actual truncation by hand.
  */
-int fuse_do_setattr(struct dentry *dentry, struct iattr *attr,
-		    struct file *file)
+int fuse_do_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
+		    struct iattr *attr, struct file *file)
 {
 	struct inode *inode = d_inode(dentry);
 	struct fuse_mount *fm = get_fuse_mount(inode);
@@ -1899,7 +1909,7 @@ int fuse_do_setattr(struct dentry *dentry, struct iattr *attr,
 	if (!fc->default_permissions)
 		attr->ia_valid |= ATTR_FORCE;
 
-	err = setattr_prepare(&nop_mnt_idmap, dentry, attr);
+	err = setattr_prepare(idmap, dentry, attr);
 	if (err)
 		return err;
 
@@ -1958,7 +1968,7 @@ int fuse_do_setattr(struct dentry *dentry, struct iattr *attr,
 
 	memset(&inarg, 0, sizeof(inarg));
 	memset(&outarg, 0, sizeof(outarg));
-	iattr_to_fattr(fc, attr, &inarg, trust_local_cmtime);
+	iattr_to_fattr(idmap, fc, attr, &inarg, trust_local_cmtime);
 	if (file) {
 		struct fuse_file *ff = file->private_data;
 		inarg.valid |= FATTR_FH;
@@ -2093,7 +2103,7 @@ static int fuse_setattr(struct mnt_idmap *idmap, struct dentry *entry,
 	if (!attr->ia_valid)
 		return 0;
 
-	ret = fuse_do_setattr(entry, attr, file);
+	ret = fuse_do_setattr(idmap, entry, attr, file);
 	if (!ret) {
 		/*
 		 * If filesystem supports acls it may have updated acl xattrs in
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 7d14d533dad1..06ff4742ab08 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2966,7 +2966,7 @@ static void fuse_do_truncate(struct file *file)
 	attr.ia_file = file;
 	attr.ia_valid |= ATTR_FILE;
 
-	fuse_do_setattr(file_dentry(file), &attr, file);
+	fuse_do_setattr(file_mnt_idmap(file), file_dentry(file), &attr, file);
 }
 
 static inline loff_t fuse_round_up(struct fuse_conn *fc, loff_t off)
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 656575e3e4cf..de0ab2f14995 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1331,8 +1331,8 @@ bool fuse_write_update_attr(struct inode *inode, loff_t pos, ssize_t written);
 int fuse_flush_times(struct inode *inode, struct fuse_file *ff);
 int fuse_write_inode(struct inode *inode, struct writeback_control *wbc);
 
-int fuse_do_setattr(struct dentry *dentry, struct iattr *attr,
-		    struct file *file);
+int fuse_do_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
+		    struct iattr *attr, struct file *file);
 
 void fuse_set_initialized(struct fuse_conn *fc);
 
-- 
2.34.1


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

* [PATCH v4 09/15] fs/fuse: drop idmap argument from __fuse_get_acl
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
                   ` (7 preceding siblings ...)
  2024-09-03 15:16 ` [PATCH v4 08/15] fs/fuse: support idmapped ->setattr op Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 10/15] fs/fuse: support idmapped ->set_acl Alexander Mikhalitsyn
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	linux-kernel

We don't need to have idmap in the __fuse_get_acl as we don't
have any use for it.

In the current POSIX ACL implementation, idmapped mounts are
taken into account on the userspace/kernel border
(see vfs_set_acl_idmapped_mnt() and vfs_posix_acl_to_xattr()).

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Reviewed-by: Christian Brauner <brauner@kernel.org>
---
 fs/fuse/acl.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/acl.c b/fs/fuse/acl.c
index 04cfd8fee992..897d813c5e92 100644
--- a/fs/fuse/acl.c
+++ b/fs/fuse/acl.c
@@ -12,7 +12,6 @@
 #include <linux/posix_acl_xattr.h>
 
 static struct posix_acl *__fuse_get_acl(struct fuse_conn *fc,
-					struct mnt_idmap *idmap,
 					struct inode *inode, int type, bool rcu)
 {
 	int size;
@@ -74,7 +73,7 @@ struct posix_acl *fuse_get_acl(struct mnt_idmap *idmap,
 	if (fuse_no_acl(fc, inode))
 		return ERR_PTR(-EOPNOTSUPP);
 
-	return __fuse_get_acl(fc, idmap, inode, type, false);
+	return __fuse_get_acl(fc, inode, type, false);
 }
 
 struct posix_acl *fuse_get_inode_acl(struct inode *inode, int type, bool rcu)
@@ -90,8 +89,7 @@ struct posix_acl *fuse_get_inode_acl(struct inode *inode, int type, bool rcu)
 	 */
 	if (!fc->posix_acl)
 		return NULL;
-
-	return __fuse_get_acl(fc, &nop_mnt_idmap, inode, type, rcu);
+	return __fuse_get_acl(fc,  inode, type, rcu);
 }
 
 int fuse_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
-- 
2.34.1


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

* [PATCH v4 10/15] fs/fuse: support idmapped ->set_acl
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
                   ` (8 preceding siblings ...)
  2024-09-03 15:16 ` [PATCH v4 09/15] fs/fuse: drop idmap argument from __fuse_get_acl Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 11/15] fs/fuse: support idmapped ->rename op Alexander Mikhalitsyn
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	linux-kernel

It's just a matter of adjusting a permission check condition
for S_ISGID flag. All the rest is already handled in the generic
VFS code.

Notice that this permission check is the analog of what
we have in posix_acl_update_mode() generic helper, but
fuse doesn't use this helper as on the kernel side we don't
care about ensuring that POSIX ACL and CHMOD permissions are in sync
as it is a responsibility of a userspace daemon to handle that.
For the same reason we don't have a calls to posix_acl_chmod(),
while most of other filesystem do.

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Reviewed-by: Christian Brauner <brauner@kernel.org>
---
 fs/fuse/acl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/fuse/acl.c b/fs/fuse/acl.c
index 897d813c5e92..8f484b105f13 100644
--- a/fs/fuse/acl.c
+++ b/fs/fuse/acl.c
@@ -144,8 +144,8 @@ int fuse_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
 		 * be stripped.
 		 */
 		if (fc->posix_acl &&
-		    !in_group_or_capable(&nop_mnt_idmap, inode,
-					 i_gid_into_vfsgid(&nop_mnt_idmap, inode)))
+		    !in_group_or_capable(idmap, inode,
+					 i_gid_into_vfsgid(idmap, inode)))
 			extra_flags |= FUSE_SETXATTR_ACL_KILL_SGID;
 
 		ret = fuse_setxattr(inode, name, value, size, 0, extra_flags);
-- 
2.34.1


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

* [PATCH v4 11/15] fs/fuse: support idmapped ->rename op
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
                   ` (9 preceding siblings ...)
  2024-09-03 15:16 ` [PATCH v4 10/15] fs/fuse: support idmapped ->set_acl Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 12/15] fs/fuse: handle idmappings properly in ->write_iter Alexander Mikhalitsyn
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	linux-kernel

RENAME_WHITEOUT is a special case of ->rename
and we need to take idmappings into account there.

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
---
v2:
	- this commit added
v4:
	- support idmapped ->rename for RENAME_WHITEOUT
---
 fs/fuse/dir.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 08bf9cc51a65..d316223bd00b 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1025,7 +1025,7 @@ static int fuse_rmdir(struct inode *dir, struct dentry *entry)
 	return err;
 }
 
-static int fuse_rename_common(struct inode *olddir, struct dentry *oldent,
+static int fuse_rename_common(struct mnt_idmap *idmap, struct inode *olddir, struct dentry *oldent,
 			      struct inode *newdir, struct dentry *newent,
 			      unsigned int flags, int opcode, size_t argsize)
 {
@@ -1046,7 +1046,7 @@ static int fuse_rename_common(struct inode *olddir, struct dentry *oldent,
 	args.in_args[1].value = oldent->d_name.name;
 	args.in_args[2].size = newent->d_name.len + 1;
 	args.in_args[2].value = newent->d_name.name;
-	err = fuse_simple_request(NULL, fm, &args);
+	err = fuse_simple_request(idmap, fm, &args);
 	if (!err) {
 		/* ctime changes */
 		fuse_update_ctime(d_inode(oldent));
@@ -1092,7 +1092,8 @@ static int fuse_rename2(struct mnt_idmap *idmap, struct inode *olddir,
 		if (fc->no_rename2 || fc->minor < 23)
 			return -EINVAL;
 
-		err = fuse_rename_common(olddir, oldent, newdir, newent, flags,
+		err = fuse_rename_common((flags & RENAME_WHITEOUT) ? idmap : NULL,
+					 olddir, oldent, newdir, newent, flags,
 					 FUSE_RENAME2,
 					 sizeof(struct fuse_rename2_in));
 		if (err == -ENOSYS) {
@@ -1100,7 +1101,7 @@ static int fuse_rename2(struct mnt_idmap *idmap, struct inode *olddir,
 			err = -EINVAL;
 		}
 	} else {
-		err = fuse_rename_common(olddir, oldent, newdir, newent, 0,
+		err = fuse_rename_common(NULL, olddir, oldent, newdir, newent, 0,
 					 FUSE_RENAME,
 					 sizeof(struct fuse_rename_in));
 	}
-- 
2.34.1


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

* [PATCH v4 12/15] fs/fuse: handle idmappings properly in ->write_iter
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
                   ` (10 preceding siblings ...)
  2024-09-03 15:16 ` [PATCH v4 11/15] fs/fuse: support idmapped ->rename op Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 13/15] fs/fuse: warn if fuse_access is called when idmapped mounts are allowed Alexander Mikhalitsyn
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	linux-kernel

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
---
v4:
	- this commit added
---
 fs/fuse/file.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 06ff4742ab08..dffc476f0bf2 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1398,6 +1398,7 @@ static void fuse_dio_unlock(struct kiocb *iocb, bool exclusive)
 static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from)
 {
 	struct file *file = iocb->ki_filp;
+	struct mnt_idmap *idmap = file_mnt_idmap(file);
 	struct address_space *mapping = file->f_mapping;
 	ssize_t written = 0;
 	struct inode *inode = mapping->host;
@@ -1412,7 +1413,7 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from)
 			return err;
 
 		if (fc->handle_killpriv_v2 &&
-		    setattr_should_drop_suidgid(&nop_mnt_idmap,
+		    setattr_should_drop_suidgid(idmap,
 						file_inode(file))) {
 			goto writethrough;
 		}
-- 
2.34.1


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

* [PATCH v4 13/15] fs/fuse: warn if fuse_access is called when idmapped mounts are allowed
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
                   ` (11 preceding siblings ...)
  2024-09-03 15:16 ` [PATCH v4 12/15] fs/fuse: handle idmappings properly in ->write_iter Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 14/15] fs/fuse: allow idmapped mounts Alexander Mikhalitsyn
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	linux-kernel

It is not possible with the current fuse code, but let's protect ourselves
from regressions in the future.

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
---
v4:
	- this commit added
---
 fs/fuse/dir.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index d316223bd00b..dd967402bf12 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1473,6 +1473,14 @@ static int fuse_access(struct inode *inode, int mask)
 
 	BUG_ON(mask & MAY_NOT_BLOCK);
 
+	/*
+	 * We should not send FUSE_ACCESS to the userspace
+	 * when idmapped mounts are enabled as for this case
+	 * we have fc->default_permissions = 1 and access
+	 * permission checks are done on the kernel side.
+	 */
+	WARN_ON_ONCE(!(fm->sb->s_iflags & SB_I_NOIDMAP));
+
 	if (fm->fc->no_access)
 		return 0;
 
-- 
2.34.1


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

* [PATCH v4 14/15] fs/fuse: allow idmapped mounts
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
                   ` (12 preceding siblings ...)
  2024-09-03 15:16 ` [PATCH v4 13/15] fs/fuse: warn if fuse_access is called when idmapped mounts are allowed Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-03 15:16 ` [PATCH v4 15/15] fs/fuse/virtio_fs: " Alexander Mikhalitsyn
  2024-09-04 15:15 ` [PATCH v4 00/15] fuse: basic support for " Miklos Szeredi
  15 siblings, 0 replies; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, Alexander Mikhalitsyn,
	linux-kernel

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.

Some extra links and examples:

- libfuse support
https://github.com/mihalicyn/libfuse/commits/idmap_support

- fuse-overlayfs support:
https://github.com/mihalicyn/fuse-overlayfs/commits/idmap_support

- cephfs-fuse conversion example
https://github.com/mihalicyn/ceph/commits/fuse_idmap

- glusterfs conversion example
https://github.com/mihalicyn/glusterfs/commits/fuse_idmap

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Reviewed-by: Christian Brauner <brauner@kernel.org>
---
v2:
	- simplified and get rid of ->allow_idmap global VFS callback
v3:
	- now use a new SB_I_NOIDMAP flag
v4:
	- small rebase changes
---
 fs/fuse/inode.c           | 12 +++++++++---
 include/uapi/linux/fuse.h | 20 +++++++++++++++++++-
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 2e26810066e8..9f9456d3e466 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1343,6 +1343,12 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 			}
 			if (flags & FUSE_NO_EXPORT_SUPPORT)
 				fm->sb->s_export_op = &fuse_export_fid_operations;
+			if (flags & FUSE_ALLOW_IDMAP) {
+				if (fc->default_permissions)
+					fm->sb->s_iflags &= ~SB_I_NOIDMAP;
+				else
+					ok = false;
+			}
 		} else {
 			ra_pages = fc->max_read / PAGE_SIZE;
 			fc->no_lock = 1;
@@ -1390,7 +1396,7 @@ void fuse_send_init(struct fuse_mount *fm)
 		FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT | FUSE_INIT_EXT |
 		FUSE_SECURITY_CTX | FUSE_CREATE_SUPP_GROUP |
 		FUSE_HAS_EXPIRE_ONLY | FUSE_DIRECT_IO_ALLOW_MMAP |
-		FUSE_NO_EXPORT_SUPPORT | FUSE_HAS_RESEND;
+		FUSE_NO_EXPORT_SUPPORT | FUSE_HAS_RESEND | FUSE_ALLOW_IDMAP;
 #ifdef CONFIG_FUSE_DAX
 	if (fm->fc->dax)
 		flags |= FUSE_MAP_ALIGNMENT;
@@ -1980,7 +1986,7 @@ static void fuse_kill_sb_anon(struct super_block *sb)
 static struct file_system_type fuse_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "fuse",
-	.fs_flags	= FS_HAS_SUBTYPE | FS_USERNS_MOUNT,
+	.fs_flags	= FS_HAS_SUBTYPE | FS_USERNS_MOUNT | FS_ALLOW_IDMAP,
 	.init_fs_context = fuse_init_fs_context,
 	.parameters	= fuse_fs_parameters,
 	.kill_sb	= fuse_kill_sb_anon,
@@ -2001,7 +2007,7 @@ static struct file_system_type fuseblk_fs_type = {
 	.init_fs_context = fuse_init_fs_context,
 	.parameters	= fuse_fs_parameters,
 	.kill_sb	= fuse_kill_sb_blk,
-	.fs_flags	= FS_REQUIRES_DEV | FS_HAS_SUBTYPE,
+	.fs_flags	= FS_REQUIRES_DEV | FS_HAS_SUBTYPE | FS_ALLOW_IDMAP,
 };
 MODULE_ALIAS_FS("fuseblk");
 
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 2ccf38181df2..f1e99458e29e 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -217,6 +217,9 @@
  *  - add backing_id to fuse_open_out, add FOPEN_PASSTHROUGH open flag
  *  - add FUSE_NO_EXPORT_SUPPORT init flag
  *  - add FUSE_NOTIFY_RESEND, add FUSE_HAS_RESEND init flag
+ *
+ *  7.41
+ *  - add FUSE_ALLOW_IDMAP
  */
 
 #ifndef _LINUX_FUSE_H
@@ -252,7 +255,7 @@
 #define FUSE_KERNEL_VERSION 7
 
 /** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 40
+#define FUSE_KERNEL_MINOR_VERSION 41
 
 /** The node ID of the root inode */
 #define FUSE_ROOT_ID 1
@@ -421,6 +424,7 @@ struct fuse_file_lock {
  * FUSE_NO_EXPORT_SUPPORT: explicitly disable export support
  * FUSE_HAS_RESEND: kernel supports resending pending requests, and the high bit
  *		    of the request ID indicates resend requests
+ * FUSE_ALLOW_IDMAP: allow creation of idmapped mounts
  */
 #define FUSE_ASYNC_READ		(1 << 0)
 #define FUSE_POSIX_LOCKS	(1 << 1)
@@ -466,6 +470,7 @@ struct fuse_file_lock {
 
 /* Obsolete alias for FUSE_DIRECT_IO_ALLOW_MMAP */
 #define FUSE_DIRECT_IO_RELAX	FUSE_DIRECT_IO_ALLOW_MMAP
+#define FUSE_ALLOW_IDMAP	(1ULL << 40)
 
 /**
  * CUSE INIT request/reply flags
@@ -984,6 +989,19 @@ struct fuse_fallocate_in {
  */
 #define FUSE_UNIQUE_RESEND (1ULL << 63)
 
+/**
+ * This value will be set by the kernel to
+ * (struct fuse_in_header).{uid,gid} fields in
+ * case when:
+ * - fuse daemon enabled FUSE_ALLOW_IDMAP
+ * - idmapping information is not available and uid/gid
+ *   can not be mapped in accordance with an idmapping.
+ *
+ * Note: an idmapping information always available
+ * for inode creation operations like:
+ * FUSE_MKNOD, FUSE_SYMLINK, FUSE_MKDIR, FUSE_TMPFILE,
+ * FUSE_CREATE and FUSE_RENAME2 (with RENAME_WHITEOUT).
+ */
 #define FUSE_INVALID_UIDGID ((uint32_t)(-1))
 
 struct fuse_in_header {
-- 
2.34.1


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

* [PATCH v4 15/15] fs/fuse/virtio_fs: allow idmapped mounts
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
                   ` (13 preceding siblings ...)
  2024-09-03 15:16 ` [PATCH v4 14/15] fs/fuse: allow idmapped mounts Alexander Mikhalitsyn
@ 2024-09-03 15:16 ` Alexander Mikhalitsyn
  2024-09-05 19:45   ` Stefan Hajnoczi
  2024-09-04 15:15 ` [PATCH v4 00/15] fuse: basic support for " Miklos Szeredi
  15 siblings, 1 reply; 22+ messages in thread
From: Alexander Mikhalitsyn @ 2024-09-03 15:16 UTC (permalink / raw)
  To: mszeredi
  Cc: brauner, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Vivek Goyal, German Maglione, Amir Goldstein, Bernd Schubert,
	Alexander Mikhalitsyn, Stefan Hajnoczi, Eugenio Pérez,
	linux-kernel, virtualization

Allow idmapped mounts for virtiofs.
It's absolutely safe as for virtiofs we have the same
feature negotiation mechanism as for classical fuse
filesystems. This does not affect any existing
setups anyhow.

virtiofsd support:
https://gitlab.com/virtio-fs/virtiofsd/-/merge_requests/245

Cc: Christian Brauner <brauner@kernel.org>
Cc: Seth Forshee <sforshee@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: German Maglione <gmaglione@redhat.com>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Bernd Schubert <bschubert@ddn.com>
Cc: <linux-fsdevel@vger.kernel.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Reviewed-by: Christian Brauner <brauner@kernel.org>
---
v3:
	- this commit added
---
 fs/fuse/virtio_fs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index dd5260141615..7e5bbaef6f76 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -1628,6 +1628,7 @@ static struct file_system_type virtio_fs_type = {
 	.name		= "virtiofs",
 	.init_fs_context = virtio_fs_init_fs_context,
 	.kill_sb	= virtio_kill_sb,
+	.fs_flags	= FS_ALLOW_IDMAP,
 };
 
 static int virtio_fs_uevent(const struct kobject *kobj, struct kobj_uevent_env *env)
-- 
2.34.1


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

* Re: [PATCH v4 00/15] fuse: basic support for idmapped mounts
  2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
                   ` (14 preceding siblings ...)
  2024-09-03 15:16 ` [PATCH v4 15/15] fs/fuse/virtio_fs: " Alexander Mikhalitsyn
@ 2024-09-04 15:15 ` Miklos Szeredi
  2024-09-04 15:29   ` Christian Brauner
  15 siblings, 1 reply; 22+ messages in thread
From: Miklos Szeredi @ 2024-09-04 15:15 UTC (permalink / raw)
  To: Alexander Mikhalitsyn
  Cc: mszeredi, brauner, stgraber, linux-fsdevel, Seth Forshee,
	Vivek Goyal, German Maglione, Amir Goldstein, Bernd Schubert,
	linux-kernel

On Tue, 3 Sept 2024 at 17:16, Alexander Mikhalitsyn
<aleksandr.mikhalitsyn@canonical.com> wrote:
>
> Dear friends,
>
> This patch series aimed to provide support for idmapped mounts
> for fuse & virtiofs. We already have idmapped mounts support for almost all
> widely-used filesystems:
> * local (ext4, btrfs, xfs, fat, vfat, ntfs3, squashfs, f2fs, erofs, ZFS (out-of-tree))
> * network (ceph)

Looks good.

Applied with some tweaks and pushed.

Thanks,
Miklos

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

* Re: [PATCH v4 03/15] fs/fuse: add an idmap argument to fuse_simple_request
  2024-09-03 15:16 ` [PATCH v4 03/15] fs/fuse: add an idmap argument to fuse_simple_request Alexander Mikhalitsyn
@ 2024-09-04 15:25   ` Christian Brauner
  0 siblings, 0 replies; 22+ messages in thread
From: Christian Brauner @ 2024-09-04 15:25 UTC (permalink / raw)
  To: Alexander Mikhalitsyn
  Cc: mszeredi, stgraber, linux-fsdevel, Seth Forshee, Miklos Szeredi,
	Amir Goldstein, Bernd Schubert, linux-kernel

On Tue, Sep 03, 2024 at 05:16:14PM GMT, Alexander Mikhalitsyn wrote:
> If idmap == NULL *and* filesystem daemon declared idmapped mounts
> support, then uid/gid values in a fuse header will be -1.
> 
> No functional changes intended.
> 
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Seth Forshee <sforshee@kernel.org>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Amir Goldstein <amir73il@gmail.com>
> Cc: Bernd Schubert <bschubert@ddn.com>
> Cc: <linux-fsdevel@vger.kernel.org>
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
> ---

Huha, you end up extending fuse_simple_request() with an idmap argument
and passing NULL 38 times and non-NULL only 4 times at the end of this
patch series. That's not pretty. Also, I really dislike passing NULL as
an argument to the idmap helpers. All of the idmapping code uses
nop_mnt_idmap for this case and I think we should the same just with
invalid_mnt_idmap constant.

So I would propose two changes:

(1) Add an extern invalid_mnt_idmap into mnt_idmapping.h and
    define it in fs/mnt_idmapping.c so that will always yield
    INVALID_VFSUID/INVALID_VFSGID. Basically, it's the same definition
    as for nop_mnt_idmap.

(2) Instead of extending fuse_simple_request() with an additional
    argument rename fuse_simple_request() to __fuse_simple_request()
    and extend __fuse_simple_request() with a struct mnt_idmap argument.

    * make fuse_simple_request() a static inline helper that calls
      __fuse_simple_request() with invalid_mnt_idmap in the fuse_i.h
      header.

    * add fuse_idmap_request() that also calls __fuse_simple_request()
      but just passes through the idmap argument.

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

* Re: [PATCH v4 00/15] fuse: basic support for idmapped mounts
  2024-09-04 15:15 ` [PATCH v4 00/15] fuse: basic support for " Miklos Szeredi
@ 2024-09-04 15:29   ` Christian Brauner
  2024-09-04 16:59     ` Miklos Szeredi
  0 siblings, 1 reply; 22+ messages in thread
From: Christian Brauner @ 2024-09-04 15:29 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Alexander Mikhalitsyn, mszeredi, stgraber, linux-fsdevel,
	Seth Forshee, Vivek Goyal, German Maglione, Amir Goldstein,
	Bernd Schubert, linux-kernel

On Wed, Sep 04, 2024 at 05:15:40PM GMT, Miklos Szeredi wrote:
> On Tue, 3 Sept 2024 at 17:16, Alexander Mikhalitsyn
> <aleksandr.mikhalitsyn@canonical.com> wrote:
> >
> > Dear friends,
> >
> > This patch series aimed to provide support for idmapped mounts
> > for fuse & virtiofs. We already have idmapped mounts support for almost all
> > widely-used filesystems:
> > * local (ext4, btrfs, xfs, fat, vfat, ntfs3, squashfs, f2fs, erofs, ZFS (out-of-tree))
> > * network (ceph)
> 
> Looks good.
> 
> Applied with some tweaks and pushed.

Ah, I didn't see your reply. Fwiw, if you agree with my suggestion then
Alex can just put that patch on top of the series or do it after we
landed it. I just think passing NULL 38 times is a bit ugly.

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

* Re: [PATCH v4 00/15] fuse: basic support for idmapped mounts
  2024-09-04 15:29   ` Christian Brauner
@ 2024-09-04 16:59     ` Miklos Szeredi
  2024-09-04 17:21       ` Aleksandr Mikhalitsyn
  0 siblings, 1 reply; 22+ messages in thread
From: Miklos Szeredi @ 2024-09-04 16:59 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Alexander Mikhalitsyn, mszeredi, stgraber, linux-fsdevel,
	Seth Forshee, Vivek Goyal, German Maglione, Amir Goldstein,
	Bernd Schubert, linux-kernel

On Wed, 4 Sept 2024 at 17:29, Christian Brauner <brauner@kernel.org> wrote:
>
> On Wed, Sep 04, 2024 at 05:15:40PM GMT, Miklos Szeredi wrote:
> > On Tue, 3 Sept 2024 at 17:16, Alexander Mikhalitsyn
> > <aleksandr.mikhalitsyn@canonical.com> wrote:
> > >
> > > Dear friends,
> > >
> > > This patch series aimed to provide support for idmapped mounts
> > > for fuse & virtiofs. We already have idmapped mounts support for almost all
> > > widely-used filesystems:
> > > * local (ext4, btrfs, xfs, fat, vfat, ntfs3, squashfs, f2fs, erofs, ZFS (out-of-tree))
> > > * network (ceph)
> >
> > Looks good.
> >
> > Applied with some tweaks and pushed.
>
> Ah, I didn't see your reply. Fwiw, if you agree with my suggestion then
> Alex can just put that patch on top of the series or do it after we
> landed it. I just think passing NULL 38 times is a bit ugly.

Yes, I agree with this comment.   I'm fine with either a redone series
or an incremental patch.

Thanks,
Miklos

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

* Re: [PATCH v4 00/15] fuse: basic support for idmapped mounts
  2024-09-04 16:59     ` Miklos Szeredi
@ 2024-09-04 17:21       ` Aleksandr Mikhalitsyn
  0 siblings, 0 replies; 22+ messages in thread
From: Aleksandr Mikhalitsyn @ 2024-09-04 17:21 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Christian Brauner, mszeredi, stgraber, linux-fsdevel,
	Seth Forshee, Vivek Goyal, German Maglione, Amir Goldstein,
	Bernd Schubert, linux-kernel

On Wed, Sep 4, 2024 at 7:00 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Wed, 4 Sept 2024 at 17:29, Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Wed, Sep 04, 2024 at 05:15:40PM GMT, Miklos Szeredi wrote:
> > > On Tue, 3 Sept 2024 at 17:16, Alexander Mikhalitsyn
> > > <aleksandr.mikhalitsyn@canonical.com> wrote:
> > > >
> > > > Dear friends,
> > > >
> > > > This patch series aimed to provide support for idmapped mounts
> > > > for fuse & virtiofs. We already have idmapped mounts support for almost all
> > > > widely-used filesystems:
> > > > * local (ext4, btrfs, xfs, fat, vfat, ntfs3, squashfs, f2fs, erofs, ZFS (out-of-tree))
> > > > * network (ceph)
> > >
> > > Looks good.
> > >
> > > Applied with some tweaks and pushed.
> >
> > Ah, I didn't see your reply. Fwiw, if you agree with my suggestion then
> > Alex can just put that patch on top of the series or do it after we
> > landed it. I just think passing NULL 38 times is a bit ugly.
>
> Yes, I agree with this comment.   I'm fine with either a redone series
> or an incremental patch.

Dear Christian,
Dear Miklos,

I'm happy to send a patch/patches on top to refactor that.

Kind regards,
Alex

>
> Thanks,
> Miklos

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

* Re: [PATCH v4 15/15] fs/fuse/virtio_fs: allow idmapped mounts
  2024-09-03 15:16 ` [PATCH v4 15/15] fs/fuse/virtio_fs: " Alexander Mikhalitsyn
@ 2024-09-05 19:45   ` Stefan Hajnoczi
  0 siblings, 0 replies; 22+ messages in thread
From: Stefan Hajnoczi @ 2024-09-05 19:45 UTC (permalink / raw)
  To: Alexander Mikhalitsyn
  Cc: mszeredi, brauner, stgraber, linux-fsdevel, Seth Forshee,
	Miklos Szeredi, Vivek Goyal, German Maglione, Amir Goldstein,
	Bernd Schubert, Eugenio Pérez, linux-kernel, virtualization

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

On Tue, Sep 03, 2024 at 05:16:26PM +0200, Alexander Mikhalitsyn wrote:
> Allow idmapped mounts for virtiofs.
> It's absolutely safe as for virtiofs we have the same
> feature negotiation mechanism as for classical fuse
> filesystems. This does not affect any existing
> setups anyhow.
> 
> virtiofsd support:
> https://gitlab.com/virtio-fs/virtiofsd/-/merge_requests/245
> 
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Seth Forshee <sforshee@kernel.org>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: German Maglione <gmaglione@redhat.com>
> Cc: Amir Goldstein <amir73il@gmail.com>
> Cc: Bernd Schubert <bschubert@ddn.com>
> Cc: <linux-fsdevel@vger.kernel.org>
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
> Reviewed-by: Christian Brauner <brauner@kernel.org>
> ---
> v3:
> 	- this commit added
> ---
>  fs/fuse/virtio_fs.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, other threads:[~2024-09-05 19:45 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-03 15:16 [PATCH v4 00/15] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 01/15] fs/namespace: introduce SB_I_NOIDMAP flag Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 02/15] fs/fuse: add basic infrastructure to support idmappings Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 03/15] fs/fuse: add an idmap argument to fuse_simple_request Alexander Mikhalitsyn
2024-09-04 15:25   ` Christian Brauner
2024-09-03 15:16 ` [PATCH v4 04/15] fs/fuse: support idmapped FUSE_EXT_GROUPS Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 05/15] fs/fuse: support idmap for mkdir/mknod/symlink/create/tmpfile Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 06/15] fs/fuse: support idmapped getattr inode op Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 07/15] fs/fuse: support idmapped ->permission " Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 08/15] fs/fuse: support idmapped ->setattr op Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 09/15] fs/fuse: drop idmap argument from __fuse_get_acl Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 10/15] fs/fuse: support idmapped ->set_acl Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 11/15] fs/fuse: support idmapped ->rename op Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 12/15] fs/fuse: handle idmappings properly in ->write_iter Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 13/15] fs/fuse: warn if fuse_access is called when idmapped mounts are allowed Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 14/15] fs/fuse: allow idmapped mounts Alexander Mikhalitsyn
2024-09-03 15:16 ` [PATCH v4 15/15] fs/fuse/virtio_fs: " Alexander Mikhalitsyn
2024-09-05 19:45   ` Stefan Hajnoczi
2024-09-04 15:15 ` [PATCH v4 00/15] fuse: basic support for " Miklos Szeredi
2024-09-04 15:29   ` Christian Brauner
2024-09-04 16:59     ` Miklos Szeredi
2024-09-04 17:21       ` Aleksandr Mikhalitsyn

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).