seccomp: apply all argument conditions - #2220
Conversation
kolyshkin
left a comment
kolyshkin
left a comment
There was a problem hiding this comment.
Review of the seccomp argument handling. Three findings, one of them a behaviour change beyond the stated fix.
Note: this comment was generated by an AI assistant (Claude) at the request of a reviewer; please double-check it.
| { | ||
| size_t r; | ||
| const size_t args_len = seccomp->syscalls[i]->args_len; | ||
| cleanup_free struct scmp_arg_cmp *arg_cmp = xmalloc (sizeof (*arg_cmp) * args_len); |
There was a problem hiding this comment.
args_len can be 0 here: libocispec sets syscalls[i]->args to a non-NULL calloc(len + 1, ...) even for an empty JSON array (libocispec/src/ocispec/runtime_spec_schema_defs_linux.c:525), so a config containing "args": [] takes the else branch of args == NULL with args_len == 0. xmalloc(0) calls malloc(0), which the C standard permits to return NULL, and xmalloc then calls OOM() and aborts the runtime.
Beyond that the allocation is unnecessary: this branch is only reached when multiple_args is false, which means every index is distinct and < 6, so args_len <= 6 is provable -- the previous fixed struct scmp_arg_cmp arg_cmp[6] was both correct and cheaper. The suggestion in the other comment goes back to the fixed array.
Note: this comment was generated by an AI assistant (Claude) at the request of a reviewer; please double-check it.
a1900d3 to
d4d7de4
Compare
remove the arbitrary limit of 6 conditions. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
d4d7de4 to
9d15f51
Compare
Harden the seccomp cache key. The cache is keyed by a checksum of the configuration, but some inputs that change the generated filter were missing or ambiguously encoded, so in principle a stale filter could be reused. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
|
LGTM |
|
@kolyshkin fine to merge this? |
3 participants
remove the arbitrary limit of 6 conditions.
@eriksjolund PTAL