| Message ID | 5538c72ca7c1bf2eb0ff3dbaa73903869ba47e95.1729209889.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | btrfs/012: fix false alerts when SELinux is enabled | expand |
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@suse.com> > Link: https://bugzilla.suse.com/show_bug.cgi?id=1231524 > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > 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. ------- 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"
在 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@suse.com> >> Link: https://bugzilla.suse.com/show_bug.cgi?id=1231524 >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> 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. 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" > >
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@suse.com> > > > Link: https://bugzilla.suse.com/show_bug.cgi?id=1231524 > > > Signed-off-by: Qu Wenruo <wqu@suse.com> > > > --- > > > 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 :) 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" > > > > > >
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@suse.com> >>>> Link: https://bugzilla.suse.com/show_bug.cgi?id=1231524 >>>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>>> --- >>>> 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@oracle.com> 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? 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" >>> >>> >> >> >
在 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@suse.com> >>>>> Link: https://bugzilla.suse.com/show_bug.cgi?id=1231524 >>>>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>>>> --- >>>>> 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@oracle.com> > > > > 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" >>>> >>>> >>> >>> >> >
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="" + # Create & populate an ext4 filesystem $MKFS_EXT4_PROG -F -b $BLOCK_SIZE $SCRATCH_DEV > $seqres.full 2>&1 || \ _notrun "Could not create ext4 filesystem"
[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@suse.com> Link: https://bugzilla.suse.com/show_bug.cgi?id=1231524 Signed-off-by: Qu Wenruo <wqu@suse.com> --- tests/btrfs/012 | 5 +++++ 1 file changed, 5 insertions(+)