Repository navigation
uucore: look up the -Z context of a relative name by its absolute name - #15336
abendrothj wants to merge 1 commit into
Conversation
Merging this PR will regress 1 benchmark
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | factor_five_38_bit_primes |
1.6 s | 2 s | -17.03% |
| ⚡ | Simulation | factor_three_39_bit_primes |
567 ms | 291.9 ms | +94.26% |
| ⚡ | Simulation | factor_thirteen_39_bit_primes |
9.5 s | 9 s | +5.85% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing abendrothj:uucore-fscreate-relative-lookup (203d45c) with main (c76f272)
Footnotes
-
70 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
|
|
||
| /// Like `canonicalize`, but a directory that does not exist yet, as for one about to be | ||
| /// created, is resolved through its parents with its own name kept. | ||
| fn canonicalize_existing(directory: &Path) -> std::io::Result<PathBuf> { |
There was a problem hiding this comment.
the name says existing but it's for dirs that don't exist yet, could you rename it?
There was a problem hiding this comment.
Done, it's now canonicalize_allow_missing, in #15333.
| ("branch/../quokka", directory.join("quokka")), | ||
| ("branch/..", directory.clone()), | ||
| (".", directory.clone()), | ||
| // Parents that are about to be created, as for `install -d -Z a/b/c`. |
There was a problem hiding this comment.
this part is for install, not mkdir (mkdir -p creates one level at a time)
please move it to the install PR with a test in tests/by-util/test_install.rs
There was a problem hiding this comment.
Done, moved to #15333 with a test in test_install.rs.
| // name must get the same label as the absolute one. | ||
| #[test] | ||
| #[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))] | ||
| fn test_selinux_default_context_relative_name() { |
There was a problem hiding this comment.
mknod and mkfifo go through the same code, please add a test for one of them too
There was a problem hiding this comment.
Done, added one for mkfifo.
| } | ||
| let scene = TestScenario::new(util_name!()); | ||
| let at = &scene.fixtures; | ||
| // A type the policy won't hand out here, so a missed lookup shows as inheritance. |
There was a problem hiding this comment.
as you said, this passes without the fix under /tmp, so CI won't catch a regression
could you use a dir the policy has a default for?
There was a problem hiding this comment.
Done. The tests now run in target/tmp, which is under the home dir in the SELinux job, so the policy has a default there. They fail without the fix in a Fedora VM set up like that job, and skip where the policy has no default.
b933376 to
203d45c
Compare
Closes #15335
FsCreateContextasked the policy for the default context of the name as given. The policy lists absolute names only, so a relative name matched nothing, the miss counted as "no default", and the new object kept the type it inherited from its parent. The name now goes throughabsolute_for_policy_lookup, asset_selinux_security_contextalready does forcp -Z.Checked as root on Fedora 44 (enforcing) against GNU 9.12:
mkdir -Zwith a relative name, with-p, and with an absolute name now gets the same type. Addedtest_selinux_default_context_relative_namefor mkdir and one for mkfifo. They run in cargo'starget/tmp, where the SELinux CI job's policy has a default, and fail without the change there; where there's no default they skip.