seccomp: add error path tests, simplify action/operator lookup - #2205
Open
kolyshkin wants to merge 2 commits into
Open
seccomp: add error path tests, simplify action/operator lookup#2205kolyshkin wants to merge 2 commits into
kolyshkin wants to merge 2 commits into
Conversation
The code paths that reject an unknown seccomp action, argument comparison operator, or filter flag had no test coverage at all, which is how the error leak fixed in commit 4e86239 (PR containers#2023) went unnoticed for so long. Add six tests covering both failure branches of get_seccomp_action() and get_seccomp_operator() -- an unknown name, and a name lacking the SCMP_ACT_/SCMP_CMP_ prefix -- plus an unknown SECCOMP_FILTER_FLAG_. Each asserts that the container is rejected *and* that the error message names the offending value, so that a regression in either the detection or the reporting is caught. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
get_seccomp_action() and get_seccomp_operator() spend five lines per name on an if/assign/return block, which made them grow to 122 lines between them for what is a plain string-to-value mapping. Replace the if chains with static tables. The #ifdef guards for the optional actions now wrap a single table entry each rather than a five-line block, and the conditional compilation is much easier to follow. SCMP_ACT_ERRNO and SCMP_ACT_TRACE keep their own lookups since they embed errno_ret and so are not constants. While at it, use has_prefix() -- already used a few lines below for "SCMP_ARCH_" -- instead of strncmp() with a hardcoded 9, and derive the amount to skip from the prefix itself. No functional change: both functions were verified to return the same value and the same status as before for every valid name and for a set of malformed ones. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.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
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-up to #2023.
Adds the missing test coverage for the seccomp error paths, then replaces the if/strcmp chains in
get_seccomp_action()andget_seccomp_operator()with static tables. No functional change.cc @eriksjolund