| Message ID | 20250224111014.2276072-1-neelx@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] fstests: btrfs/314: fix the failure when SELinux is enabled | expand |
On Mon, 24 Feb 2025 at 12:10, Daniel Vacek <neelx@suse.com> 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. And of course I forgot to remove this sentence. Please, remove it if you decide to merge this fix. > Signed-off-by: Daniel Vacek <neelx@suse.com> > --- > tests/btrfs/314 | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/tests/btrfs/314 b/tests/btrfs/314 > index 76dccc41..29111ece 100755 > --- a/tests/btrfs/314 > +++ b/tests/btrfs/314 > @@ -38,7 +38,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 $(_common_dev_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 >
On Mon, Feb 24, 2025 at 12:10:14PM +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@suse.com> > --- > tests/btrfs/314 | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/tests/btrfs/314 b/tests/btrfs/314 > index 76dccc41..29111ece 100755 > --- a/tests/btrfs/314 > +++ b/tests/btrfs/314 > @@ -38,7 +38,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 $(_common_dev_mount_options) ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt} I note that there are several similar instances of this in common/, either as '_mount $(_common_dev_mount_options)' or as '$MOUNT_PROG -t $FSTYP `_common_dev_mount_options $*`' That kinda says to me there should be a _mount_dev() wrapper, similar to the _mkfs_dev() wrapper like this: _mount_dev() { _mount $(_common_dev_mount_options) $* } And all these open coded device mounts be converted to use the wrapper. That way we don't have this problem of omitting _common_dev_mount_options when future tests open code specific device mounts. -Dave.
On Tue, Feb 25, 2025 at 09:01:06AM +1100, Dave Chinner wrote: > On Mon, Feb 24, 2025 at 12:10:14PM +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@suse.com> > > --- > > tests/btrfs/314 | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/tests/btrfs/314 b/tests/btrfs/314 > > index 76dccc41..29111ece 100755 > > --- a/tests/btrfs/314 > > +++ b/tests/btrfs/314 > > @@ -38,7 +38,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 $(_common_dev_mount_options) ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt} > > I note that there are several similar instances of this in common/, > either as '_mount $(_common_dev_mount_options)' or as '$MOUNT_PROG > -t $FSTYP `_common_dev_mount_options $*`' > > That kinda says to me there should be a _mount_dev() wrapper, > similar to the _mkfs_dev() wrapper like this: > > _mount_dev() > { > _mount $(_common_dev_mount_options) $* > } > > And all these open coded device mounts be converted to use the > wrapper. That way we don't have this problem of omitting > _common_dev_mount_options when future tests open code specific device > mounts. OK, so we have two requirements about mount/umount now: 1) Use _mount_dev to replace _mount or even $MOUNT_PROG. But overlayfs tests might be special, he has to deal with underlying devices which is not suitable for $(_common_dev_mount_options). 2) Use _umount to replace $UMOUNT_PROG: https://lore.kernel.org/fstests/20250221041819.GX21799@frogsfrogsfrogs/ Thanks, Zorro > > -Dave. > -- > Dave Chinner > david@fromorbit.com >
On Mon, Feb 24, 2025 at 12:10:14PM +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@suse.com> > --- > tests/btrfs/314 | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/tests/btrfs/314 b/tests/btrfs/314 > index 76dccc41..29111ece 100755 > --- a/tests/btrfs/314 > +++ b/tests/btrfs/314 > @@ -38,7 +38,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 $(_common_dev_mount_options) ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt} This's good to me, Reviewed-by: Zorro Lang <zlang@redhat.com> > > $XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' ${src}/foo | _filter_xfs_io > _btrfs subvolume snapshot -r ${src} ${src}/snap1 > -- > 2.48.1 >
On 24/2/25 19:35, Daniel Vacek wrote: > On Mon, 24 Feb 2025 at 12:10, Daniel Vacek <neelx@suse.com> 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 >> ... >> It’s actually good that the Btrfs receive failed because the send had an unlabeled security context—kind of a validation, even though it wasn’t intentional. The fix here fits the objective of the test case. Reviewed-by: Anand Jain <anand.jain@oracle.com> >> 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. > > And of course I forgot to remove this sentence. Please, remove it if > you decide to merge this fix. Fixed the changelog and pushed it (for-next). Thanks, Anand > >> Signed-off-by: Daniel Vacek <neelx@suse.com> >> --- >> tests/btrfs/314 | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/tests/btrfs/314 b/tests/btrfs/314 >> index 76dccc41..29111ece 100755 >> --- a/tests/btrfs/314 >> +++ b/tests/btrfs/314 >> @@ -38,7 +38,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 $(_common_dev_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 >>
On Fri, 28 Feb 2025 at 07:10, Anand Jain <anand.jain@oracle.com> wrote: > > On 24/2/25 19:35, Daniel Vacek wrote: > > On Mon, 24 Feb 2025 at 12:10, Daniel Vacek <neelx@suse.com> 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 > >> ... > >> > > It’s actually good that the Btrfs receive failed because the send had > an unlabeled security context—kind of a validation, even though it > wasn’t intentional. The fix here fits the objective of the test case. It's the other way around. Send (missing the mount option) had the label (as expected) which was refused by the receive side due to the explicit context mount option. That's how SELinux is designed. > Reviewed-by: Anand Jain <anand.jain@oracle.com> > > >> 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. > > > > And of course I forgot to remove this sentence. Please, remove it if > > you decide to merge this fix. > > Fixed the changelog and pushed it (for-next). > > Thanks, Anand > > > > > > >> Signed-off-by: Daniel Vacek <neelx@suse.com> > >> --- > >> tests/btrfs/314 | 6 +++++- > >> 1 file changed, 5 insertions(+), 1 deletion(-) > >> > >> diff --git a/tests/btrfs/314 b/tests/btrfs/314 > >> index 76dccc41..29111ece 100755 > >> --- a/tests/btrfs/314 > >> +++ b/tests/btrfs/314 > >> @@ -38,7 +38,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 $(_common_dev_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 > >> >
On Fri, 28 Feb 2025 at 07:10, Anand Jain <anand.jain@oracle.com> wrote:
> Fixed the changelog and pushed it (for-next).
Thank you.
diff --git a/tests/btrfs/314 b/tests/btrfs/314 index 76dccc41..29111ece 100755 --- a/tests/btrfs/314 +++ b/tests/btrfs/314 @@ -38,7 +38,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 $(_common_dev_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
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@suse.com> --- tests/btrfs/314 | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)