On Tue, May 02, 2023 at 06:23:51PM +0000, Chuck Lever III wrote:
>
>
> > On May 2, 2023, at 12:50 PM, Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote:
> >
> >
> >
> >> On May 2, 2023, at 9:49 AM, Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote:
> >>
> >>>
> >>> On May 2, 2023, at 9:36 AM, Christian Brauner <brauner@xxxxxxxxxx> wrote:
> >>>
> >>> We've aligned setgid behavior over multiple kernel releases. The details
> >>> can be found in commit cf619f891971 ("Merge tag 'fs.ovl.setgid.v6.2' of
> >>> git://git.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping") and
> >>> commit 426b4ca2d6a5 ("Merge tag 'fs.setgid.v6.0' of
> >>> git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux").
> >>> Consistent setgid stripping behavior is now encapsulated in the
> >>> setattr_should_drop_sgid() helper which is used by all filesystems that
> >>> strip setgid bits outside of vfs proper. Usually ATTR_KILL_SGID is
> >>> raised in e.g., chown_common() and is subject to the
> >>> setattr_should_drop_sgid() check to determine whether the setgid bit can
> >>> be retained. Since nfsd is raising ATTR_KILL_SGID unconditionally it
> >>> will cause notify_change() to strip it even if the caller had the
> >>> necessary privileges to retain it. Ensure that nfsd only raises
> >>> ATR_KILL_SGID if the caller lacks the necessary privileges to retain the
> >>> setgid bit.
> >>>
> >>> Without this patch the setgid stripping tests in LTP will fail:
> >>>
> >>>> As you can see, the problem is S_ISGID (0002000) was dropped on a
> >>>> non-group-executable file while chown was invoked by super-user, while
> >>>
> >>> [...]
> >>>
> >>>> fchown02.c:66: TFAIL: testfile2: wrong mode permissions 0100700, expected 0102700
> >>>
> >>> [...]
> >>>
> >>>> chown02.c:57: TFAIL: testfile2: wrong mode permissions 0100700, expected 0102700
> >>>
> >>> With this patch all tests pass.
> >>>
> >>> Reported-by: Sherry Yang <sherry.yang@xxxxxxxxxx>
> >>> Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>
> >>
> >> There are some similar fstests failures this fix might address.
> >>
> >> I've applied this patch to the nfsd-fixes tree for broader
> >> testing.
> >
> > ERROR: modpost: "setattr_should_drop_sgid" [fs/nfsd/nfsd.ko] undefined!
> >
> > Did I apply this patch to the wrong kernel?
>
> setattr_should_drop_sgid() is not available to callers built as
> modules. It needs an EXPORT_SYMBOL or _GPL.
Hey Chuck,
Thanks for taking a look!
The required export is part of
commit 4f704d9a835 ("nfs: use vfs setgid helper")
which is in current mainline as of Monday this week which was part of my
v6.4/vfs.misc PR that was merged.
So this should all work fine on mainline. Seems I didn't use --base
which is why that info was missing.
Thanks!
Christian
