On Thu, Feb 20, 2025 at 03:57:23PM +0100, Daniel Vacek wrote:
> When SELinux is enabled this test fails unable to receive a file with
> security label attribute:
>
> --- tests/btrfs/314.out
> +++ results//btrfs/314.out.bad
> @@ -17,5 +17,6 @@
> At subvol TEST_DIR/314/tempfsid_mnt/snap1
> Receive SCRATCH_MNT
> At subvol snap1
> +ERROR: lsetxattr foo security.selinux=unconfined_u:object_r:unlabeled_t:s0 failed: Operation not supported
> Send: 42d69d1a6d333a7ebdf64792a555e392 TEST_DIR/314/tempfsid_mnt/foo
> -Recv: 42d69d1a6d333a7ebdf64792a555e392 SCRATCH_MNT/snap1/foo
> +Recv: d41d8cd98f00b204e9800998ecf8427e SCRATCH_MNT/snap1/foo
> ...
>
> Setting the security label file attribute fails due to the default mount
> option implied by fstests:
>
> MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sdb /mnt/scratch
>
> See commit 3839d299 ("xfstests: mount xfs with a context when selinux is on")
>
> fstests by default mount test and scratch devices with forced SELinux
> context to get rid of the additional file attributes when SELinux is
> enabled. When a test mounts additional devices from the pool, it may need
> to honor this option to keep on par. Otherwise failures may be expected.
>
> Moreover this test is perfectly fine labeling the files so let's just
> disable the forced context for this one.
>
> Signed-off-by: Daniel Vacek <neelx@xxxxxxxx>
> ---
Take it easy, Thanks for both of you would like to help fstests to get
better :)
Firstly, SELINUX_MOUNT_OPTIONS isn't a parameter to enable or disable
SELinux test. We just use it to avoid tons of ondisk selinux lables to
mess up the testing. So mount with a specified SELINUX_MOUNT_OPTIONS
to avoid new ondisk selinux labels always be created in each file's
extended attributes field.
Secondly, I don't want to attend the argument :) Just for this patch review,
I prefer just doing:
_scratch_mount
- _mount ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt}
+ _mount $SELINUX_MOUNT_OPTIONS ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt}
or if you concern MOUNT_OPTIONS and SELINUX_MOUNT_OPTIONS both, maybe:
_scratch_mount
- _mount ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt}
+ _mount $(_common_dev_mount_options) ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt}
That's enough to help "_scratch_mount" and later "_mount" use same
SELINUX_MOUNT_OPTIONS, and fix the test failure you hit.
About resetting "SELINUX_MOUNT_OPTIONS", I think btrfs/314 isn't a test case
cares about SELinux labels on-disk or not. So how about don't touch it.
If you'd like to talk about if xfstests cases should test with a specified
SELINUX_MOUNT_OPTIONS mount option or not, you can send another patch to talk
about 3839d299 ("xfstests: mount xfs with a context when selinux is on").
Now let's fix this failure at first :)
Thanks,
Zorro
> tests/btrfs/314 | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tests/btrfs/314 b/tests/btrfs/314
> index 76dccc41..cc1a2264 100755
> --- a/tests/btrfs/314
> +++ b/tests/btrfs/314
> @@ -21,6 +21,10 @@ _cleanup()
>
> . ./common/filter.btrfs
>
> +# Disable the forced SELinux context. We are fine testing the
> +# security labels with this test when SELinux is enabled.
> +SELINUX_MOUNT_OPTIONS=
> +
> _require_scratch_dev_pool 2
> _require_btrfs_fs_feature temp_fsid
>
> @@ -38,7 +42,7 @@ send_receive_tempfsid()
> # Use first 2 devices from the SCRATCH_DEV_POOL
> mkfs_clone ${SCRATCH_DEV} ${SCRATCH_DEV_NAME[1]}
> _scratch_mount
> - _mount ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt}
> + _mount ${SELINUX_MOUNT_OPTIONS} ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt}
>
> $XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' ${src}/foo | _filter_xfs_io
> _btrfs subvolume snapshot -r ${src} ${src}/snap1
> --
> 2.48.1
>
>
