Re: [PATCH] generic: update setgid tests

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Tue, Jan 03, 2023 at 04:12:09PM -0800, Darrick J. Wong wrote:
> On Tue, Jan 03, 2023 at 10:31:20PM +0100, Christian Brauner wrote:
> > On Tue, Jan 03, 2023 at 09:47:37AM -0800, Darrick J. Wong wrote:
> > > On Tue, Jan 03, 2023 at 04:28:20PM +0100, Christian Brauner wrote:
> > > > Over mutiple kernel releases we have reworked setgid inheritance
> > > > significantly due to long-standing security issues, security issues that
> > > > were reintroduced after they were fixed, and the subtle and difficult
> > > > inheritance rules that plagued individual filesystems. We have lifted
> > > > setgid inheritance into the VFS proper in earlier patches. Starting with
> > > > kernel v6.2 we have made setgid inheritance consistent between the write
> > > > and setattr (ch{mod,own}) paths.
> > > > 
> > > > The gist of the requirement is that in order to inherit the setgid bit
> > > > the user needs to be in the group of the file or have CAP_FSETID in
> > > > their user namespace. Otherwise the setgid bit will be dropped
> > > > irregardless of the file's executability. Change the tests accordingly
> > > > and annotate them with the commits that changed the behavior.
> > > > 
> > > > Note, that only with v6.2 setgid inheritance works correctly for
> > > > overlayfs in the write path. Before this the setgid bit was always
> > > > retained.
> > > > 
> > > > Link: https://lore.kernel.org/linux-ext4/CAOQ4uxhmCgyorYVtD6=n=khqwUc=MPbZs+y=sqt09XbGoNm_tA@xxxxxxxxxxxxxx
> > > > Link: https://lore.kernel.org/linux-fsdevel/20221212112053.99208-1-brauner@xxxxxxxxxx
> > > > Link: https://lore.kernel.org/linux-fsdevel/20221122142010.zchf2jz2oymx55qi@wittgenstein
> > > > Cc: Amir Goldstein <amir73il@xxxxxxxxx>
> > > > Cc: Zorro Lang <zlang@xxxxxxxxxx>
> > > > Signed-off-by: Christian Brauner (Microsoft) <brauner@xxxxxxxxxx>
> > > > ---
> > > >  tests/generic/673     | 12 +++++++++---
> > > >  tests/generic/673.out |  8 ++++----
> > > >  tests/generic/683     | 11 ++++++++---
> > > >  tests/generic/683.out |  8 ++++----
> > > >  tests/generic/684     | 11 ++++++++---
> > > >  tests/generic/684.out |  8 ++++----
> > > >  tests/generic/685     | 11 ++++++++---
> > > >  tests/generic/685.out |  8 ++++----
> > > >  8 files changed, 49 insertions(+), 28 deletions(-)
> > > > 
> > > > diff --git a/tests/generic/673 b/tests/generic/673
> > > > index 6d1f49ea..1d8e4184 100755
> > > > --- a/tests/generic/673
> > > > +++ b/tests/generic/673
> > > > @@ -23,6 +23,12 @@ _require_scratch_reflink
> > > >  _scratch_mkfs >> $seqres.full
> > > >  _scratch_mount
> > > >  _require_congruent_file_oplen $SCRATCH_MNT 1048576
> > > > +
> > > > +# Due to multiple security issues and potential for subtle bugs around setgid
> > > > +# inheritance the rules in the write and chmod/chown paths have been made
> > > > +# consistent and are enforced by the VFS since kernel 6.2.
> > > > +_fixed_in_kernel_version "v6.2"
> > > 
> > > Uh... will this new behavior get ported to the LTS kernels too, or are
> > 
> > I plan on backporting this but I would like to wait for a little while
> > longer into the v6.2 cycle before doing this as everyone is just coming
> > out of the holiday season trying to catch up.
> > 
> > > we simply changing the kernel behavior here?
> > 
> > Yes, this is a behavioral change. I made sure to mention the risks and
> > reasons involved on the list along the patches and in the pull requests
> > that were merged.
> 
> <nod> Ok.  I'm cool with this so long as everyone else seems to be. :)
> 
> > > Also, this patch does correct the new test failures that I saw as soon
> > > as I pulled 6.2-rc1, but it doesn't fix the finsert and fcollapse tests:
> > 
> > Indeed I have missed them because they were skipped:
> > 
> > generic/686       [not run] xfs_io finsert  failed (old kernel/wrong fs?)
> > generic/687       [not run] xfs_io fcollapse  failed (old kernel/wrong fs?)
> > 
> > I'll update the series accordingly. Thanks for pointing that out.
> 
> Ok, thank you!  And really thank you for digging through the hairy
> setgid inheritance mess. :)

Thanks for the kind words. Always appreciated! :)
And thanks for all the help with the xfs patches over the last years.

Christian



[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux