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@xxxxxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
Reviewed-by: Christian Brauner <christian.brauner@xxxxxxxxxx>
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
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
