mkdir: warn when --context is used without SELinux/SMACK#13311
mkdir: warn when --context is used without SELinux/SMACK#13311Chaganti-Reddy wants to merge 3 commits into
Conversation
|
GNU testsuite comparison: |
|
|
||
| #[test] | ||
| #[cfg(all( | ||
| feature = "feat_selinux", |
There was a problem hiding this comment.
Running a uutils compiled with feat_selinux requires an SELinux enabled Kernel at run time, so I'm not sure what this is testing.
There was a problem hiding this comment.
It's testing that when the binary has selinux support compiled in but the kernel doesn't have it enabled, mkdir still creates the dir and just warns, matching GNU. That's a real state, feat_selinux only means libselinux is linked in, it doesn't need an SELinux kernel to run. Checked it here, getenforce says Disabled, and the test actually hit the assertions instead of skipping.
Though you're right it never runs in our own CI. The only job that tests with feat_selinux boots a Fedora VM with SELinux on, the lint job with the feature only runs clippy. So this test always self-skips on our runners even though it's correct locally. Can drop it if you'd rather not carry something CI never exercises.
There was a problem hiding this comment.
I'd assumed it wasn't a real state due to this comment:
Line 85 in 18e1df7
I guess the comment is incorrect.
This does expose a wider issue if we don't run CI for feat_selinux without SELinux kernel.
There was a problem hiding this comment.
Makes sense that's where that came from, the comment was just wrong. Pushed a fix for it.
Agreed on the CI gap too, that's real. Feels like its own thing separate from this PR though — want me to open an issue for it, or would you rather just add a plain ubuntu-latest feat_selinux test job here directly?
There was a problem hiding this comment.
Yeah it applies, and it's worse there. is_smack_enabled() is the same kind of safe runtime check (Path::new("/sys/fs/smackfs").exists()), comment was wrong the same way. But set_smack_label_and_cleanup was also silently returning Ok when SMACK isn't enabled, no warning at all, so mkdir was just pretending --context worked. Fixed the comment and wired the SMACK branch to warn the same way SELinux does now, reusing the existing mkdir-warning-context-not-selinux string since it already says "SELinux/SMACK" generically. Also feat_smack isn't built in any CI job at all currently, not even for lint, so this had zero coverage anywhere. Added a test for it locally, confirmed it hits the real assertion here.
7d767c4 to
03108d5
Compare
|
opened #13332 for the CI coverage gap |
the selinux check happened before even trying to set the context, so on a kernel without SELinux/SMACK the whole block was skipped and --context silently did nothing. GNU still creates the dir but warns about it. added the missing else branch, matches GNU's wording now.
Fixes #12985