fix: match the root at a path component boundary when transforming paths - #1955
Open
vyncint wants to merge 1 commit into
Open
fix: match the root at a path component boundary when transforming paths#1955vyncint wants to merge 1 commit into
vyncint wants to merge 1 commit into
Conversation
The root transformer used a plain string prefix comparison to determine whether a path is contained in the (from) root. A path that shares a textual prefix with the root but lies outside it was therefore treated as being under the root and rewritten, corrupting the path. For a root of /run/nvidia/driver and a target root of /host: /run/nvidia/driver-backup/lib.so -> /host/-backup/lib.so /run/nvidia/driverfoo -> /host/foo Both paths are outside the root and must be left unchanged. The prefix match is now required to end at a path component boundary. A path equal to the root, and a root with a trailing separator (including a root of /), are handled as before. This applies to both the host and container root transformers since they share transformPath, and is reachable from 'nvidia-ctk cdi transform root --from/--to' as well as from the driver root transform applied during CDI spec generation. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
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
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.
Problem
pkg/nvcdi/transform/rootdecides whether a path is contained in the (from) root using a plain string prefix comparison:A path that shares a textual prefix with the root but is not actually inside it therefore matches, and is rewritten into a corrupted path.
With
root=/run/nvidia/driverandtargetRoot=/host:/run/nvidia/driver/lib/libcuda.so/host/lib/libcuda.so/host/lib/libcuda.so/run/nvidia/driver-backup/lib/libcuda.so/host/-backup/lib/libcuda.so/run/nvidia/driverfoo/host/fooThe last two paths lie outside the root and must be left untouched.
Reachability
transformPathis shared byhostRootTransformerandcontainerRootTransformer, so both host and container paths are affected. The roots are user-supplied:nvidia-ctk cdi transform root --from <root> --to <target>NewDriverTransformer, driven by--driver-root/--dev-root)Fix
Require the prefix match to end at a path component boundary. Behaviour that was already correct is unchanged:
/, still matchesTesting
Added
TestTransformPathcovering the two boundary cases plus five cases for the existing behaviour. I verified that the two new cases fail without the fix and pass with it, and that the other five pass either way.Locally, on a machine with no NVIDIA GPU:
make test— pass (44 packages)golangci-lint run ./...— 0 issuesgolangci-lint fmt --diff ./...— cleango build ./...— passCoverage for
pkg/nvcdi/transform/root: 89.9% -> 90.2%.Out of scope
The same prefix-comparison pattern also appears in
internal/lookup/root.RelativeToRoot,internal/discover/mounts.go, andpkg/nvcdi/driver-nvml.go. I have left those untouched to keep this change small and reviewable, but I am happy to follow up if you would like them addressed as well.