learn: follow-ups from the #209 review - #220
Merged
Merged
Conversation
canonicalize_or_keep rewrote "/proc/self" to "/proc/<pid>" but only mapped results back when they carried a trailing "/<pid>/" prefix, so an open of /proc/self itself came out as a numeric-pid path and was dropped as junk. Use Path::strip_prefix on both sides so the bare directory and its entries round-trip the same way, and stop matching unrelated names that merely start with "/proc/self". canonicalize_parent_or_keep still resolved against the supervisor's own /proc/self, so rename or unlink targets under /proc/self/cwd landed on the wrong process. Route it through the same pid-aware helper. The exe test only asserted that /proc/self/exe was absent, which a dropped read would also satisfy; it now checks the resolved interpreter path is present. Signed-off-by: Cong Wang <cwang@multikernel.io>
The direct-write, ancestor-walk, and read filters each spelled the "/" guard differently (byte compare vs Path compare), which hides that they are the same rule. Signed-off-by: Cong Wang <cwang@multikernel.io>
The paragraph said a warning is printed for direct writes to Protected or Guarded paths, but collapse_write_paths prints a NOTE and still records the path. Also drop the trailing whitespace. Signed-off-by: Cong Wang <cwang@multikernel.io>
Contributor
|
Lgtm |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups to #209 that were left out of that PR as minor.
canonicalize_or_keeprewrote/proc/selfto/proc/<pid>but only mapped results back when they carried a trailing/<pid>/prefix, so an open of/proc/selfitself came out as a numeric-pid path and was dropped as junk. Both sides now usePath::strip_prefix, so the bare directory and its entries round-trip the same way, and unrelated names that merely start with/proc/selfno longer match.canonicalize_parent_or_keepstill resolved against the supervisor's own/proc/self; it now goes through the same pid-aware helper, so rename and unlink targets under/proc/self/cwdland on the workload's process./proc/self/exetest only asserted that the alias was absent, which a dropped read would also satisfy. It now requires the resolved interpreter path./guards in learn shared one rule spelled two ways; they now call a singleis_fs_roothelper. This only affectssandlock learnoutput, notrunenforcement.docs/learn.mdsaid a warning is printed for direct writes to Protected or Guarded paths; the code prints a NOTE and still records the path.Tests:
cargo test -p sandlock-cli --test learn_test -- --test-threads=4passes (40/40), including a newtest_proc_self_bare_preservedthat fails on main.