linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Aleksandr Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
To: Christian Brauner <brauner@kernel.org>
Cc: mszeredi@redhat.com, stgraber@stgraber.org,
	linux-fsdevel@vger.kernel.org,
	 Seth Forshee <sforshee@kernel.org>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Vivek Goyal <vgoyal@redhat.com>,
	 German Maglione <gmaglione@redhat.com>,
	Amir Goldstein <amir73il@gmail.com>,
	 Bernd Schubert <bschubert@ddn.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 00/11] fuse: basic support for idmapped mounts
Date: Fri, 16 Aug 2024 15:45:35 +0200	[thread overview]
Message-ID: <CAEivzxe2fdT1fPhbT4XUWLsR7LyTxv5oUmExyrq7QP4QfeDWyw@mail.gmail.com> (raw)
In-Reply-To: <CAEivzxf2qH8XBXA2a+U4bQeeVC+eB8m9tDC08jxT=trFEzLpTA@mail.gmail.com>

On Fri, Aug 16, 2024 at 10:58 AM Aleksandr Mikhalitsyn
<aleksandr.mikhalitsyn@canonical.com> wrote:
>
> On Fri, Aug 16, 2024 at 10:02 AM Christian Brauner <brauner@kernel.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@kernel.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@ubuntu:/home/ubuntu/xfstests-dev# git log | head -n 3
> commit f5ada754d5838d29fd270257003d0d123a9d1cd2
> Author: Darrick J. Wong <djwong@kernel.org>
> Date:   Fri Jul 26 09:51:07 2024 -0700
>
> root@ubuntu:/home/ubuntu/xfstests-dev# cat local.config
> export TEST_DEV=myfs
> export TEST_DIR=/mnt/test
> export FSTYP=virtiofs
>
> root@ubuntu:/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

  reply	other threads:[~2024-08-16 13:45 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-15  9:24 [PATCH v3 00/11] fuse: basic support for idmapped mounts Alexander Mikhalitsyn
2024-08-15  9:24 ` [PATCH v3 01/11] fs/namespace: introduce SB_I_NOIDMAP flag Alexander Mikhalitsyn
2024-08-15  9:24 ` [PATCH v3 02/11] fs/fuse: add FUSE_OWNER_UID_GID_EXT extension Alexander Mikhalitsyn
2024-08-15  9:24 ` [PATCH v3 03/11] fs/fuse: support idmap for mkdir/mknod/symlink/create Alexander Mikhalitsyn
2024-08-15  9:24 ` [PATCH v3 04/11] fs/fuse: support idmapped getattr inode op Alexander Mikhalitsyn
2024-08-15  9:24 ` [PATCH v3 05/11] fs/fuse: support idmapped ->permission " Alexander Mikhalitsyn
2024-08-15  9:24 ` [PATCH v3 06/11] fs/fuse: support idmapped ->setattr op Alexander Mikhalitsyn
2024-08-15  9:24 ` [PATCH v3 07/11] fs/fuse: drop idmap argument from __fuse_get_acl Alexander Mikhalitsyn
2024-08-15  9:24 ` [PATCH v3 08/11] fs/fuse: support idmapped ->set_acl Alexander Mikhalitsyn
2024-08-15  9:24 ` [PATCH v3 09/11] fs/fuse: properly handle idmapped ->rename op Alexander Mikhalitsyn
2024-08-15  9:24 ` [PATCH v3 10/11] fs/fuse: allow idmapped mounts Alexander Mikhalitsyn
2024-08-15  9:24 ` [PATCH v3 11/11] fs/fuse/virtio_fs: " Alexander Mikhalitsyn
2024-08-16  8:01 ` [PATCH v3 00/11] fuse: basic support for " Christian Brauner
2024-08-16  8:58   ` Aleksandr Mikhalitsyn
2024-08-16 13:45     ` Aleksandr Mikhalitsyn [this message]
2024-08-19 11:34 ` Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAEivzxe2fdT1fPhbT4XUWLsR7LyTxv5oUmExyrq7QP4QfeDWyw@mail.gmail.com \
    --to=aleksandr.mikhalitsyn@canonical.com \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=bschubert@ddn.com \
    --cc=gmaglione@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=mszeredi@redhat.com \
    --cc=sforshee@kernel.org \
    --cc=stgraber@stgraber.org \
    --cc=vgoyal@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).