Re: [PATCH] xfs: fix up non-directory creation in SGID directories

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Jan 14, 2021 at 11:45:11AM +0100, Christian Brauner wrote:
> On Wed, Jan 13, 2021 at 07:46:30PM +0100, Christoph Hellwig wrote:
> > XFS always inherits the SGID bit if it is set on the parent inode, while
> > the generic inode_init_owner does not do this in a few cases where it can
> > create a possible security problem, see commit 0fa3ecd87848
> > ("Fix up non-directory creation in SGID directories") for details.
> > 
> > Switch XFS to use the generic helper for the normal path to fix this,
> > just keeping the simple field inheritance open coded for the case of the
> > non-sgid case with the bsdgrpid mount option.
> > 
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Reported-by: Christian Brauner <christian.brauner@ubuntu.com>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> 
> Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>
> 
> I ran the idmapped mounts xfstests on this patchset. With this patch
> applied I was able to remove the special casing for xfs (apart from the
> irix compatibility check) and got clean test runs:
> 
> 
> 1. with regular setgid inheritance rules
> root@f2-vm:/xfstests# ./check generic/622

Is this test posted to fstests somewhere?

FWIW the code change looks reasonable to me, but I wanted to see the
functionality exercise test first. :)

--D

> FSTYP         -- xfs (non-debug)
> PLATFORM      -- Linux/x86_64 f2-vm 5.11.0-rc3-brauner-idmapped-mounts-xfs #311 SMP Thu Jan 14 09:55:14 UTC 2021
> MKFS_OPTIONS  -- -f -bsize=4096 /dev/loop7
> MOUNT_OPTIONS -- /dev/loop7 /mnt/scratch
> 
> generic/622 1s ...  2s
> Ran: generic/622
> Passed all 1 tests
> 
> 2. with irix_sgid_inherit setgid inheritance rules
> root@f2-vm:/xfstests# echo 1 > /proc/sys/fs/xfs/irix_sgid_inherit
> root@f2-vm:/xfstests# ./check generic/622
> FSTYP         -- xfs (non-debug)
> PLATFORM      -- Linux/x86_64 f2-vm 5.11.0-rc3-brauner-idmapped-mounts-xfs #311 SMP Thu Jan 14 09:55:14 UTC 2021
> MKFS_OPTIONS  -- -f -bsize=4096 /dev/loop7
> MOUNT_OPTIONS -- /dev/loop7 /mnt/scratch
> 
> generic/622 2s ...  1s
> Ran: generic/622
> Passed all 1 tests
> 
> Thanks!
> Christian