在 2025/2/21 22:23, Daniel Vacek 写道:
On Fri, 21 Feb 2025 at 10:48, Qu Wenruo <quwenruo.btrfs@xxxxxxx> wrote:
在 2025/2/21 19:43, Daniel Vacek 写道:
On Fri, 21 Feb 2025 at 09:20, Qu Wenruo <wqu@xxxxxxxx> wrote:
[...]
Remember, user can provide their own mount options (including the
SELinux ones) through MOUNT_OPTIONS environmental variables.
So you at least need a full reason why SElinux context must be disable
for this case.
And I see none.
It does not need to be disabled. But it also does not have to be
enabled for this test.
Then good lucky if one day some QA guy finds out the send/receive
behavior has a SELINUX specific bug, and you need to explain to them why
it's a good idea to not test SELinux for this particular workload.
Your mindset of "XX feature doesn't have to be tested" makes nosense for
a test suite.
Huh, this is a huge misunderstanding.You still got it the other way
around. I'm sorry if I was confusing you.
I never said I'm against testing SELinux. Quite the opposite, to be
honest. I did argue that testing the default system policy without
overriding the context also works for this test, at least with my
testing on the latest TW.
SELinux is always tested. Just either with the default system policy
or with the given forced context.
At least not with the default policy on the latest Tumbleweed I was testing on.
But your mileage may vary, I guess.
Or do you use such an option with any of your mounts? I doubt so.
Check the mentioned commit 3839d299. fstests cripple SELinux by
default. Which doesn't look good by itself.
Do you really believe that commit is going crippling SELINUX?
Well, in a way, yes.
What it does is that it overrides the system's default policy. Which
may make sense, as for example your system policy may deny some
operations the tests do, eventually resulting in failed tests.
Though as a side-effect it also prevents writing the security label
file attribute by design with the mount option override. In such a
case SELinux just returns with -EOPNOTSUPP.
3213 sbsec = selinux_superblock(inode->i_sb);
3214 if (!(sbsec->flags & SBLABEL_MNT))
3215 return -EOPNOTSUPP;
... ^^^^^^^^^^^^^^^^^^^
This only explains why your mount option fix is correct.
Nope. This triggers precisely when the context is forced with the
mount option. In that case setting the file attribute is not
supported.
BS, overriding the SELINUX context just means disabling SELINUX.
Your "fix" from the very beginning is doing two conflicting things:
- Disable SELINUX context by overriding it
- Add back the empty SELinux context for mount
Explain why it makes any sense?
The send stream has SELinux attrs, that's because the original fs is
mounted with SELinux context (the regular _scratch_mount() helper added
SELinux context).
Nope. Again, the other way around. The send stream has the
`security.selinux` attribute precisely because the mount was missing
the option and hence the file labels were used (and not refused).
There is no SELinux xattrs because you damn disabled SELinux for this
test case!!
But
the receive side fails as that mount actually did use the context
mount option and so it was refusing setting the file label returning
this -EOPNOTSUPP error.
But later we manually mounted a btrfs, not using _scratch_mount(), thus
the new mounted btrfs doesn't have SELinux context, thus unable to set
the SELinux attrs at receive side.
This is just wrong.
Whatever you believe, I think I have wasted too much time on this non-sense.
It doesn't show why you need both the mount fix and overriding SELINUX
context at all.
I'm saying you don't need both from the very beginning. Where did I say you do?
Then why damn putting the override and mount fix in the same damn patch?
