On Sun, Feb 23, 2025 at 07:56:02AM +1030, Qu Wenruo wrote:
>
>
> 在 2025/2/22 19:07, Zorro Lang 写道:
> > 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.
>
> Thanks for the sane final call.
>
> Exactly what I want in the first place.
Your suggestion is good to me, but I think you confused Daniel a bit with "overriding
the SELINUX context just means disabling SELINUX" :) Anyway, I believe both of you
know how to fix this issue and how SELinux works, maybe you're just not talking in the
same channel, so take it easy, let's over it and move on :-D
Thanks,
Zorro
>
> Thanks,
> Qu
>
> >
> > 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
> > >
> > >
> >
> >
>
