Re: [PATCH] fstests: btrfs/314: fix the failure when SELinux is enabled

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]





在 2025/2/21 18:10, Daniel Vacek 写道:
On Thu, 20 Feb 2025 at 22:54, Qu Wenruo <wqu@xxxxxxxx> wrote:



在 2025/2/21 08:06, Qu Wenruo 写道:


在 2025/2/21 01:27, Daniel Vacek 写道:
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>

Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>

Thanks,
Qu

---
   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=

Wait for a minute, this means you're disabling SELINUX mount options
completely.

I'm not sure if this is really needed.
+
   _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}

The problem of the old code is it doesn't have any SELinux related mount
option, thus later receive will fail to set SELinux context.

But since you have already added SELINUX_MOUNT_OPTIONS, I think you do
not need to disable the SELINUX_MOUNT_OPTIONS.

Have you tested with only this change, without resetting
SELINUX_MOUNT_OPTIONS?

Yes, I tested both. Actually resetting this option was the first fix I
came up with, that's why I kept it. This option breaks the test case
when SELinux is enabled.
But then I figured the other way around (using the option consistently
with all the mounts) also works. So I added it for consistency.
At that point, resetting the option is not really strictly needed
anymore (as you correctly suspect).

So there are two possible solutions. Each one makes the testcase 314
pass. But they can as well be combined.

Resetting the SELINUX one is not a good solution, that just means we reduce the coverage (No more SELINUX coverage for this test case anymore).

So please only fix the mount command (with the extra selinux mount option), without overriding the existing SElinux config.

Thanks,
Qu

Anyways, this test is fine without forcing the default mount context,
which is more a bandaid for other fstests, IIUC. There's no need for
this option in this case, at least with my testing. Hence I disabled
it. Does it fail for you?

Thanks,
Qu
       $XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' ${src}/foo |
_filter_xfs_io
       _btrfs subvolume snapshot -r ${src} ${src}/snap1








[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux