On Wed, Jun 22, 2022 at 7:41 PM Darrick J. Wong <djwong@xxxxxxxxxx> wrote: > > On Fri, Jun 17, 2022 at 01:06:41PM +0300, Amir Goldstein wrote: > > From: "Darrick J. Wong" <djwong@xxxxxxxxxx> > > > > commit e014f37db1a2d109afa750042ac4d69cf3e3d88e upstream. > > > > [remove userns argument of setattr_copy() for backport] > > > > Filipe Manana pointed out that XFS' behavior w.r.t. setuid/setgid > > revocation isn't consistent with btrfs[1] or ext4. Those two > > filesystems use the VFS function setattr_copy to convey certain > > attributes from struct iattr into the VFS inode structure. > > > > Andrey Zhadchenko reported[2] that XFS uses the wrong user namespace to > > decide if it should clear setgid and setuid on a file attribute update. > > This is a second symptom of the problem that Filipe noticed. > > > > XFS, on the other hand, open-codes setattr_copy in xfs_setattr_mode, > > xfs_setattr_nonsize, and xfs_setattr_time. Regrettably, setattr_copy is > > /not/ a simple copy function; it contains additional logic to clear the > > setgid bit when setting the mode, and XFS' version no longer matches. > > > > The VFS implements its own setuid/setgid stripping logic, which > > establishes consistent behavior. It's a tad unfortunate that it's > > scattered across notify_change, should_remove_suid, and setattr_copy but > > XFS should really follow the Linux VFS. Adapt XFS to use the VFS > > functions and get rid of the old functions. > > > > [1] https://lore.kernel.org/fstests/CAL3q7H47iNQ=Wmk83WcGB-KBJVOEtR9+qGczzCeXJ9Y2KCV25Q@xxxxxxxxxxxxxx/ > > [2] https://lore.kernel.org/linux-xfs/20220221182218.748084-1-andrey.zhadchenko@xxxxxxxxxxxxx/ > > > > Fixes: 7fa294c8991c ("userns: Allow chown and setgid preservation") > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> > > Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx> > > Reviewed-by: Christoph Hellwig <hch@xxxxxx> > > Reviewed-by: Christian Brauner <brauner@xxxxxxxxxx> > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > > Same question as I posted to Leah's series -- have all the necessary VFS > fixes and whatnot been backported to 5.10? Such that all the new sgid > inheritance tests actually pass with this patch applied? :) The only patch I backorted to 5.10 is: xfs: fix up non-directory creation in SGID directories I will check which SGID tests ran on my series. Personally, I would rather defer THIS patch to a later post to stable (Leah's patch as well) until we have a better understanding of the state of SGID issues. Thanks, Amir.
