Message-ID: <CAEivzxe2fdT1fPhbT4XUWLsR7LyTxv5oUmExyrq7QP4QfeDWyw@mail.gmail.com>
Date: Fri, 16 Aug 2024 15:45:35 +0200
From: Aleksandr Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
To: Christian Brauner <brauner@...nel.org>
Cc: mszeredi@...hat.com, stgraber@...raber.org, linux-fsdevel@...r.kernel.org, 
	Seth Forshee <sforshee@...nel.org>, Miklos Szeredi <miklos@...redi.hu>, Vivek Goyal <vgoyal@...hat.com>, 
	German Maglione <gmaglione@...hat.com>, Amir Goldstein <amir73il@...il.com>, 
	Bernd Schubert <bschubert@....com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 00/11] fuse: basic support for idmapped mounts

On Fri, Aug 16, 2024 at 10:58 AM Aleksandr Mikhalitsyn
<aleksandr.mikhalitsyn@...onical.com> wrote:
>
> On Fri, Aug 16, 2024 at 10:02 AM Christian Brauner <brauner@...nel.org> wrote:
> >
> > On Thu, Aug 15, 2024 at 11:24:17AM GMT, Alexander Mikhalitsyn 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)
> > >
> > > Git tree (based on torvalds/master):
> > > v3: https://github.com/mihalicyn/linux/commits/fuse_idmapped_mounts.v3
> > > current: https://github.com/mihalicyn/linux/commits/fuse_idmapped_mounts
> > >
> > > 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:
> > > 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
> > > flags FUSE_OWNER_UID_GID_EXT and 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
> >
> > So I have no further comments on this and from my perspective this is:
> >
> > Reviewed-by: Christian Brauner <brauner@...nel.org>
>
> Thanks, Christian! ;-)
>
> >
> > I would really like to see tests for this feature as this is available
> > to unprivileged users.
>
> Sure. I can confirm that this thing passes xfstests for virtiofs.
>
> My setup:
>
> - host machine
>
> Virtiofsd options:
>
> [ virtiofsd sources from
> https://gitlab.com/virtio-fs/virtiofsd/-/merge_requests/245 ]
> ./target/debug/virtiofsd --socket-path=/tmp/vfsd.sock --shared-dir
> /home/alex/Documents/dev/tmp --announce-submounts
> --inode-file-handles=mandatory --posix-acl
>
> QEMU options:
>         -object memory-backend-memfd,id=mem,size=$RAM,share=on \
>         -numa node,memdev=mem \
>         -chardev socket,id=char0,path=/tmp/vfsd.sock \
>         -device vhost-user-fs-pci,queue-size=1024,chardev=char0,tag=myfs \
>
> - guest
>
> xfstests version:
>
> root@...ntu:/home/ubuntu/xfstests-dev# git log | head -n 3
> commit f5ada754d5838d29fd270257003d0d123a9d1cd2
> Author: Darrick J. Wong <djwong@...nel.org>
> Date:   Fri Jul 26 09:51:07 2024 -0700
>
> root@...ntu:/home/ubuntu/xfstests-dev# cat local.config
> export TEST_DEV=myfs
> export TEST_DIR=/mnt/test
> export FSTYP=virtiofs
>
> root@...ntu:/home/ubuntu/xfstests-dev# ./check -g idmapped
> FSTYP         -- virtiofs
> PLATFORM      -- Linux/x86_64 ubuntu 6.11.0-rc3+ #2 SMP
> PREEMPT_DYNAMIC Fri Aug 16 10:23:41 CEST 2024
>
> generic/633 1s ...  0s
> generic/644 0s ...  1s
> generic/645 18s ...  18s
> generic/656       [not run] fsgqa user not defined.
> generic/689       [not run] fsgqa user not defined.
> generic/696       [not run] this test requires a valid $SCRATCH_DEV
> generic/697 0s ...  1s
> generic/698       [not run] this test requires a valid $SCRATCH_DEV
> generic/699       [not run] this test requires a valid $SCRATCH_DEV
> Ran: generic/633 generic/644 generic/645 generic/656 generic/689
> generic/696 generic/697 generic/698 generic/699
> Not run: generic/656 generic/689 generic/696 generic/698 generic/699
> Passed all 9 tests
>
> I'll try to do more tests, for example with fuse-overlayfs and get
> back with results.

Ok, it wasn't smooth to make xfstests to run with overlayfs-fuse.

It only started to live after I commented out a bunch of checks in
_check_if_dev_already_mounted/_check_mounted_on:
https://git.kernel.org/pub/scm/linux/kernel/git/brauner/xfstests-dev.git/tree/common/rc#n1613
https://git.kernel.org/pub/scm/linux/kernel/git/brauner/xfstests-dev.git/tree/common/rc#n1635
https://git.kernel.org/pub/scm/linux/kernel/git/brauner/xfstests-dev.git/tree/common/rc#n1644

I think we have some space for improvements for xfstests+fuse combination. :-)

$ cat /sbin/mount.fuse.overlayfs
#!/bin/bash
ulimit -n 1048576
exec /mnt/fuse-overlayfs/fuse-overlayfs -o $4 $1 $2

$ cat local.config
export TEST_DEV=non1
export TEST_DIR=/mnt2
export FSTYP=fuse
export FUSE_SUBTYP=.overlayfs
export MOUNT_OPTIONS="-olowerdir=/home/ubuntu/fuse_tmp/scratch_lower,upperdir=/home/ubuntu/fuse_tmp/scratch_upper,workdir=/home/ubuntu/fuse_tmp/scratch_work,allow_other,default_permissions"
export TEST_FS_MOUNT_OPTS="-olowerdir=/home/ubuntu/fuse_tmp/lower,upperdir=/home/ubuntu/fuse_tmp/upper,workdir=/home/ubuntu/fuse_tmp/work,allow_other,default_permissions"

================== without idmapped mounts support ====================

# ./check -g idmapped
FSTYP         -- fuse
PLATFORM      -- Linux/x86_64 ubuntu 6.11.0-rc3+ #2 SMP
PREEMPT_DYNAMIC Fri Aug 16 10:23:41 CEST 2024

generic/633 0s ... [failed, exit status 1]- output mismatch (see
/home/ubuntu/xfstests-dev/results//generic/633.out.bad)
    --- tests/generic/633.out    2023-06-07 12:19:04.309062045 +0000
    +++ /home/ubuntu/xfstests-dev/results//generic/633.out.bad
2024-08-16 13:30:20.471569848 +0000
    @@ -1,2 +1,4 @@
     QA output created by 633
     Silence is golden
    +vfstest.c: 1561: setgid_create - Success - failure: is_setgid
    +vfstest.c: 2418: run_test - Success - failure: create operations
in directories with setgid bit set
    ...
    (Run 'diff -u /home/ubuntu/xfstests-dev/tests/generic/633.out
/home/ubuntu/xfstests-dev/results//generic/633.out.bad'  to see the
entire diff)
generic/644 0s ... [not run] vfstest not support by fuse
generic/645 10s ... [not run] vfstest not support by fuse
generic/656 0s ... [not run] vfstest not support by fuse
generic/689 0s ... [not run] vfstest not support by fuse
generic/696       [not run] this test requires a valid $SCRATCH_DEV
generic/697 1s ... - output mismatch (see
/home/ubuntu/xfstests-dev/results//generic/697.out.bad)
    --- tests/generic/697.out    2023-06-07 12:19:04.313062164 +0000
    +++ /home/ubuntu/xfstests-dev/results//generic/697.out.bad
2024-08-16 13:30:21.919598831 +0000
    @@ -1,2 +1,4 @@
     QA output created by 697
    +vfstest.c: 2018: setgid_create_acl - Success - failure: is_setgid
    +vfstest.c: 2418: run_test - Success - failure: create operations
in directories with setgid bit set under posix acl
     Silence is golden
    ...
    (Run 'diff -u /home/ubuntu/xfstests-dev/tests/generic/697.out
/home/ubuntu/xfstests-dev/results//generic/697.out.bad'  to see the
entire diff)

HINT: You _MAY_ be missing kernel fix:
      1639a49ccdce fs: move S_ISGID stripping into the vfs_*() helpers

generic/698       [not run] this test requires a valid $SCRATCH_DEV
generic/699       [not run] this test requires a valid $SCRATCH_DEV
Ran: generic/633 generic/644 generic/645 generic/656 generic/689
generic/696 generic/697 generic/698 generic/699
Not run: generic/644 generic/645 generic/656 generic/689 generic/696
generic/698 generic/699
Failures: generic/633 generic/697
Failed 2 of 9 tests

================== with idmapped mounts support ====================

# ./check -g idmapped
FSTYP         -- fuse
PLATFORM      -- Linux/x86_64 ubuntu 6.11.0-rc3+ #2 SMP
PREEMPT_DYNAMIC Fri Aug 16 10:23:41 CEST 2024

generic/633 0s ... [failed, exit status 1]- output mismatch (see
/home/ubuntu/xfstests-dev/results//generic/633.out.bad)
    --- tests/generic/633.out    2023-06-07 12:19:04.309062045 +0000
    +++ /home/ubuntu/xfstests-dev/results//generic/633.out.bad
2024-08-16 13:29:30.358557063 +0000
    @@ -1,2 +1,4 @@
     QA output created by 633
     Silence is golden
    +vfstest.c: 1561: setgid_create - Success - failure: is_setgid
    +vfstest.c: 2418: run_test - Success - failure: create operations
in directories with setgid bit set
    ...
    (Run 'diff -u /home/ubuntu/xfstests-dev/tests/generic/633.out
/home/ubuntu/xfstests-dev/results//generic/633.out.bad'  to see the
entire diff)
generic/644 0s ...  0s
generic/645 10s ...  10s
generic/656        0s
generic/689        0s
generic/696       [not run] this test requires a valid $SCRATCH_DEV
generic/697 1s ... - output mismatch (see
/home/ubuntu/xfstests-dev/results//generic/697.out.bad)
    --- tests/generic/697.out    2023-06-07 12:19:04.313062164 +0000
    +++ /home/ubuntu/xfstests-dev/results//generic/697.out.bad
2024-08-16 13:29:41.466783240 +0000
    @@ -1,2 +1,4 @@
     QA output created by 697
    +vfstest.c: 2018: setgid_create_acl - Success - failure: is_setgid
    +vfstest.c: 2418: run_test - Success - failure: create operations
in directories with setgid bit set under posix acl
     Silence is golden
    ...
    (Run 'diff -u /home/ubuntu/xfstests-dev/tests/generic/697.out
/home/ubuntu/xfstests-dev/results//generic/697.out.bad'  to see the
entire diff)

HINT: You _MAY_ be missing kernel fix:
      1639a49ccdce fs: move S_ISGID stripping into the vfs_*() helpers

generic/698       [not run] this test requires a valid $SCRATCH_DEV
generic/699       [not run] this test requires a valid $SCRATCH_DEV
Ran: generic/633 generic/644 generic/645 generic/656 generic/689
generic/696 generic/697 generic/698 generic/699
Not run: generic/696 generic/698 generic/699
Failures: generic/633 generic/697
Failed 2 of 9 tests

As we can see it's clearly not related to idmapped mounts, as I
compare two cases, overlayfs-fuse compiled *without* support for
idmapped
mounts and *with*.

>
> Kind regards,
> Alex

Powered by blists - more mailing lists