From: Christian Brauner <brauner@kernel.org>
To: Trond Myklebust <trondmy@hammerspace.com>
Cc: "suy.fnst@fujitsu.com" <suy.fnst@fujitsu.com>,
"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
"l@damenly.su" <l@damenly.su>
Subject: Re: [bug?] nfs setgid inheritance
Date: Wed, 23 Feb 2022 17:09:26 +0100 [thread overview]
Message-ID: <20220223160926.iidztq5nf3wssw4m@wittgenstein> (raw)
In-Reply-To: <be3b341c4f0cf177b78f55de70cdb3a15ed808f4.camel@hammerspace.com>
On Wed, Feb 23, 2022 at 12:24:02PM +0000, Trond Myklebust wrote:
> On Wed, 2022-02-23 at 09:44 +0100, Christian Brauner wrote:
> > On Sat, Feb 19, 2022 at 05:00:18PM +0000, Trond Myklebust wrote:
> > > On Sat, 2022-02-19 at 12:34 +0100, Christian Brauner wrote:
> > > > On Sat, Feb 19, 2022 at 08:34:30AM +0000,
> > > > suy.fnst@fujitsu.com wrote:
> > > > > Hi NFS folks,
> > > > > During our xfstests, we found generic/633 fails like:
> > > > > ============================================
> > > > > FSTYP -- nfs
> > > > > PLATFORM -- Linux/x86_64 btrfs 5.17.0-rc4-custom #236 SMP
> > > > > PREEMPT Sat Feb 19 15:09:03 CST 2022
> > > > > MKFS_OPTIONS -- 127.0.0.1:/nfsscratch
> > > > > MOUNT_OPTIONS -- -o vers=4 127.0.0.1:/nfsscratch /mnt/scratch
> > > > >
> > > > > generic/633 0s ... [failed, exit status 1]- output mismatch
> > > > > (see
> > > > > /root/xfstests-dev/results//generic/633.out.bad)
> > > > > --- tests/generic/633.out 2021-05-23 14:03:08.879999997
> > > > > +0800
> > > > > +++ /root/xfstests-dev/results//generic/633.out.bad 2022-
> > > > > 02-19
> > > > > 16:31:28.660000013 +0800
> > > > > @@ -1,2 +1,4 @@
> > > > > QA output created by 633
> > > > > Silence is golden
> > > > > +idmapped-mounts.c: 7906: setgid_create - Success -
> > > > > failure:
> > > > > is_setgid
> > > > > +idmapped-mounts.c: 13907: run_test - Success - failure:
> > > > > create
> > > > > operations in directories with setgid bit set
> > > > > ...
> > > > > (Run 'diff -u /root/xfstests-dev/tests/generic/633.out
> > > > > /root/xfstests-dev/results//generic/633.out.bad' to see the
> > > > > entire
> > > > > diff)
> > > > > Ran: generic/633
> > > > > Failures: generic/633
> > > > > Failed 1 of 1 tests
> > > > > ============================================
> > > > >
> > > > > The failed test is about setgid inheritance.
> > > > > When a file is created with S_ISGID in the directory with
> > > > > S_ISGID,
> > > > > NFS doesn't strip the setgid bit of the new created file but
> > > > > others
> > > > > (ext4/xfs/btrfs) do. They call inode_init_owner() which does
> > > > > the strip after new_inode().
> > > > > However, NFS has its own logical to handle inode capacities.
> > > > > As the test says the behavior can be filesystem type specific,
> > > > > I'd report to you NFS guys and ask whether it's a bug or not?
> > > >
> > > > Thanks for the report. I'm not sure why NFS would have different
> > > > rules
> > > > for setgid inheritance. So I'm inclined to think that this is
> > > > simply
> > > > a
> > > > bug similar to what we saw in xfs and ceph. But I'll let the NFS
> > > > folks
> > > > determine that.
> > > >
> > > > XFS is only special in so far as it has a sysctl that lets it
> > > > alter
> > > > sgid
> > > > inheritance behavior. And it only has that to allow for legacy
> > > > irix
> > > > semantics iiuc.
> > >
> > > OK, so how do you expect this 'idmapped-mounts' functionality to
> > > work
> > > on NFS? I'm not asking about this bug in particular. I'm asking
> > > about
> > > what this is supposed to do in general.
> >
> > Just to clarify, the bug has nothing to do with idmapped mounts. The
> > idmapped mount testsuite always had a vfs generic part. That vfs
> > generic
> > part has been made available to all filesystems supporting xfstests a
> > few weeks ago. (So far this setgid inheritance bug here has been
> > present
> > in 3 filesystems.)
>
> The setgid stuff works just fine with regular use, when the server is
> able to determine when to clear the bit. It only breaks with this kind
> of test where the server is being lied to by the client's upper layers.
I think you misunderstand: it is not possible to create idmapped mounts
for a mounted NFS client. In order for a filesystem to support idmapped
mounts it must set FS_ALLOW_IDMAP which currently only btrfs, ext4, fat,
and xfs do. The failing test does not use idmapped mounts in any way.
next prev parent reply other threads:[~2022-02-23 16:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-19 8:34 [bug?] nfs setgid inheritance suy.fnst
2022-02-19 11:34 ` Christian Brauner
2022-02-19 17:00 ` Trond Myklebust
2022-02-23 8:44 ` Christian Brauner
2022-02-23 12:24 ` Trond Myklebust
2022-02-23 16:09 ` Christian Brauner [this message]
2022-02-24 8:52 ` Christian Brauner
2022-02-24 9:06 ` Su Yue
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=20220223160926.iidztq5nf3wssw4m@wittgenstein \
--to=brauner@kernel.org \
--cc=l@damenly.su \
--cc=linux-nfs@vger.kernel.org \
--cc=suy.fnst@fujitsu.com \
--cc=trondmy@hammerspace.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).