Re: [PATCH] btrfs/012: fix false alerts when SELinux is enabled

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





在 2024/10/23 19:30, Anand Jain 写道:


On 23/10/24 12:12, Zorro Lang wrote:
On Tue, Oct 22, 2024 at 01:12:15PM +1030, Qu Wenruo wrote:


在 2024/10/19 08:45, Anand Jain 写道:
On 18/10/24 08:04, Qu Wenruo wrote:
[FALSE FAILURE]
If SELinux is enabled, the test btrfs/012 will fail due to metadata
mismatch:

FSTYP         -- btrfs
PLATFORM      -- Linux/x86_64 localhost 6.4.0-150600.23.25-default #1
SMP PREEMPT_DYNAMIC Tue Oct  1 10:54:01 UTC 2024 (ea7c56d)
MKFS_OPTIONS  -- /dev/loop1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/loop1 /
mnt/scratch

btrfs/012       - output mismatch (see /home/adam/xfstests-dev/
results//btrfs/012.out.bad)
      --- tests/btrfs/012.out    2024-10-18 10:15:29.132894338 +1030
      +++ /home/adam/xfstests-dev/results//btrfs/012.out.bad
2024-10-18 10:25:51.834819708 +1030
      @@ -1,6 +1,1390 @@
       QA output created by 012
       Checking converted btrfs against the original one:
      -OK
      +metadata mismatch in /p0/d2/f4
      +metadata mismatch in /p0/d2/f5
      +metadata and data mismatch in /p0/d2/
      +metadata and data mismatch in /p0/
      ...

[CAUSE]
All the mismatch happens in the metadata, to be more especific,
it's the
security xattrs.

Although btrfs-convert properly convert all xattrs including the
security ones, at mount time we will get new SELinux labels,
causing the
mismatch between the converted and original fs.

[FIX]
Override SELINUX_MOUNT_OPTIONS so that we will not touch the security
xattrs, and that should fix the false alert.

Reported-by: Long An <lan@xxxxxxxx>
Link: https://bugzilla.suse.com/show_bug.cgi?id=1231524
Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
---
   tests/btrfs/012 | 5 +++++
   1 file changed, 5 insertions(+)

diff --git a/tests/btrfs/012 b/tests/btrfs/012
index b23e039f4c9f..5811b3b339cb 100755
--- a/tests/btrfs/012
+++ b/tests/btrfs/012
@@ -32,6 +32,11 @@ _require_extra_fs ext4
   BASENAME="stressdir"
   BLOCK_SIZE=`_get_block_size $TEST_DIR`
+# Override the SELinux mount options, or it will lead to unexpected
+# different security.selinux between the original and converted fs,
+# causing false metadata mismatch during fssum.
+export SELINUX_MOUNT_OPTIONS=""
+

SELINUX_MOUNT_OPTIONS is set only when SELinux is enabled on the
system,
so disabling SELinux will suffice.

Are you suggesting to disable SELinux just to pass the test case?

Then it doesn't sound correct to me at all.

It should be the test case to adapt to all kinds of systems, not the
other way.

Hi Anand, I think Qu is right, it's not worth disable the whole SELinux
(at the beginning of fstests running), just for a single test case.
I just hope to make sure btrfs forks agree this's a failure which should
be fixed in test side, but not change the selinux config for btrfs-progs.
If you're sure about it, I'll merge this patch :)


Yes, I realized that a bit later.

Reviewed-by: Anand Jain <anand.jain@xxxxxxxxxx>



Even if we create _require_selinux() and _reset_selinux_mount_options(),
there are only a few consumers, such as btrfs/075 and generic/700 for
the former, and btrfs/008, btrfs/019, and generic/700 for the latter.
Do you think it is better?

I think the current way of overriding SELINUX_MOUNT_OPTIONS is good enough.
There aren't that many test cases bothering xattr that carefully (except
those fssums ones).

Thanks,
Qu

Thx, Anand


Thanks,
Zorro


Thanks,
Qu


-------
fstests/common/config:
if [ -x /usr/sbin/selinuxenabled ] && /usr/sbin/selinuxenabled; then
          : ${SELINUX_MOUNT_OPTIONS:="-o context=$(stat -c %C /)"}
          export SELINUX_MOUNT_OPTIONS
fi
----------

Thanks, Anand

   # Create & populate an ext4 filesystem
   $MKFS_EXT4_PROG -F -b $BLOCK_SIZE $SCRATCH_DEV > $seqres.full
2>&1 || \
       _notrun "Could not create ext4 filesystem"












[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