From 9b36167b3805cd871deb15b1e5a5aee6b6d6a3b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Sj=C3=B6lund?= Date: Wed, 18 Mar 2026 07:56:04 +0100 Subject: [PATCH 1/2] seccomp: refactor error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix error leak. Fix incorrect assumption about err value. Closes: https://github.com/containers/crun/issues/2021 Closes: https://github.com/containers/crun/issues/2022 Co-authored-by: Kir Kolyshkin Signed-off-by: Erik Sjölund --- src/libcrun/seccomp.c | 98 ++++++++++++++++++++++++++++--------------- 1 file changed, 64 insertions(+), 34 deletions(-) diff --git a/src/libcrun/seccomp.c b/src/libcrun/seccomp.c index 4b12f22f8d..777925ab97 100644 --- a/src/libcrun/seccomp.c +++ b/src/libcrun/seccomp.c @@ -102,10 +102,10 @@ syscall_seccomp (unsigned int operation, unsigned int flags, void *args) return (int) syscall (__NR_seccomp, operation, flags, args); } -static unsigned long -get_seccomp_operator (const char *name, libcrun_error_t *err) -{ #ifdef HAVE_SECCOMP +static enum scmp_compare +get_seccomp_operator_raw (const char *name) +{ if (strcmp (name, "SCMP_CMP_NE") == 0) return SCMP_CMP_NE; if (strcmp (name, "SCMP_CMP_LT") == 0) @@ -120,18 +120,23 @@ get_seccomp_operator (const char *name, libcrun_error_t *err) return SCMP_CMP_GT; if (strcmp (name, "SCMP_CMP_MASKED_EQ") == 0) return SCMP_CMP_MASKED_EQ; + return _SCMP_CMP_MIN; // Error. +} + +static int +get_seccomp_operator (const char *name, enum scmp_compare *op, libcrun_error_t *err) +{ + *op = get_seccomp_operator_raw (name); + + if (*op == _SCMP_CMP_MIN) + return crun_make_error (err, 0, "seccomp get operator `%s`", name); - crun_make_error (err, 0, "seccomp get operator `%s`", name); - return 0; -#else return 0; -#endif } -static unsigned long long -get_seccomp_action (const char *name, int errno_ret, libcrun_error_t *err) +static int +get_seccomp_action (const char *name, int errno_ret, uint32_t *action, libcrun_error_t *err) { -#ifdef HAVE_SECCOMP const char *p; p = name; @@ -141,39 +146,63 @@ get_seccomp_action (const char *name, int errno_ret, libcrun_error_t *err) p += 9; if (strcmp (p, "ALLOW") == 0) - return SCMP_ACT_ALLOW; + { + *action = SCMP_ACT_ALLOW; + return 0; + } else if (strcmp (p, "ERRNO") == 0) - return SCMP_ACT_ERRNO (errno_ret); + { + *action = SCMP_ACT_ERRNO (errno_ret); + return 0; + } else if (strcmp (p, "KILL") == 0) - return SCMP_ACT_KILL; + { + *action = SCMP_ACT_KILL; + return 0; + } # ifdef SCMP_ACT_LOG else if (strcmp (p, "LOG") == 0) - return SCMP_ACT_LOG; + { + *action = SCMP_ACT_LOG; + return 0; + } # endif else if (strcmp (p, "TRAP") == 0) - return SCMP_ACT_TRAP; + { + *action = SCMP_ACT_TRAP; + return 0; + } else if (strcmp (p, "TRACE") == 0) - return SCMP_ACT_TRACE (errno_ret); + { + *action = SCMP_ACT_TRACE (errno_ret); + return 0; + } # ifdef SCMP_ACT_KILL_PROCESS else if (strcmp (p, "KILL_PROCESS") == 0) - return SCMP_ACT_KILL_PROCESS; + { + *action = SCMP_ACT_KILL_PROCESS; + return 0; + } # endif # ifdef SCMP_ACT_KILL_THREAD else if (strcmp (p, "KILL_THREAD") == 0) - return SCMP_ACT_KILL_THREAD; + { + *action = SCMP_ACT_KILL_THREAD; + return 0; + } # endif # ifdef SCMP_ACT_NOTIFY else if (strcmp (p, "NOTIFY") == 0) - return SCMP_ACT_NOTIFY; + { + *action = SCMP_ACT_NOTIFY; + return 0; + } # endif fail: - crun_make_error (err, 0, "seccomp get action `%s`", name); - return 0; -#else - return 0; -#endif + return crun_make_error (err, 0, "seccomp get action `%s`", name); } +#endif static void make_lowercase (char *str) @@ -666,7 +695,8 @@ libcrun_generate_seccomp (struct libcrun_seccomp_gen_ctx_s *gen_ctx, libcrun_err int ret; size_t i; cleanup_seccomp scmp_filter_ctx ctx = NULL; - int action, default_action, default_errno_value = EPERM; + int default_errno_value = EPERM; + uint32_t action, default_action; const char *def_action = NULL; /* The bpf filter was loaded from the cache, nothing to do here. */ @@ -696,9 +726,9 @@ libcrun_generate_seccomp (struct libcrun_seccomp_gen_ctx_s *gen_ctx, libcrun_err default_errno_value = seccomp->default_errno_ret; } - default_action = get_seccomp_action (def_action, default_errno_value, err); - if (UNLIKELY (err && *err != NULL)) - return -1; + ret = get_seccomp_action (def_action, default_errno_value, &default_action, err); + if (UNLIKELY (ret < 0)) + return ret; ctx = seccomp_init (default_action); if (ctx == NULL) @@ -741,9 +771,9 @@ libcrun_generate_seccomp (struct libcrun_seccomp_gen_ctx_s *gen_ctx, libcrun_err errno_ret = seccomp->syscalls[i]->errno_ret; } - action = get_seccomp_action (seccomp->syscalls[i]->action, errno_ret, err); - if (UNLIKELY (err && *err != NULL)) - return -1; + ret = get_seccomp_action (seccomp->syscalls[i]->action, errno_ret, &action, err); + if (UNLIKELY (ret < 0)) + return ret; if (action == default_action) continue; @@ -795,9 +825,9 @@ libcrun_generate_seccomp (struct libcrun_seccomp_gen_ctx_s *gen_ctx, libcrun_err char *op = seccomp->syscalls[i]->args[k]->op; arg_cmp[k].arg = seccomp->syscalls[i]->args[k]->index; - arg_cmp[k].op = get_seccomp_operator (op, err); - if (arg_cmp[k].op == 0) - return crun_make_error (err, 0, "get_seccomp_operator"); + ret = get_seccomp_operator (op, &(arg_cmp[k].op), err); + if (UNLIKELY (ret < 0)) + return ret; arg_cmp[k].datum_a = seccomp->syscalls[i]->args[k]->value; arg_cmp[k].datum_b = seccomp->syscalls[i]->args[k]->value_two; } From 232137d71fac386b0d329b475fc641950a019f36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Sj=C3=B6lund?= Date: Wed, 18 Mar 2026 07:56:36 +0100 Subject: [PATCH 2/2] seccomp: optimize common prefix comparison MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Suggested-by: Kir Kolyshkin Signed-off-by: Erik Sjölund --- src/libcrun/seccomp.c | 102 ++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 54 deletions(-) diff --git a/src/libcrun/seccomp.c b/src/libcrun/seccomp.c index 777925ab97..b319581fdc 100644 --- a/src/libcrun/seccomp.c +++ b/src/libcrun/seccomp.c @@ -106,20 +106,30 @@ syscall_seccomp (unsigned int operation, unsigned int flags, void *args) static enum scmp_compare get_seccomp_operator_raw (const char *name) { - if (strcmp (name, "SCMP_CMP_NE") == 0) + const char *p; + + p = name; + if (strncmp (p, "SCMP_CMP_", 9)) + goto fail; + + p += 9; + + if (strcmp (p, "NE") == 0) return SCMP_CMP_NE; - if (strcmp (name, "SCMP_CMP_LT") == 0) + if (strcmp (p, "LT") == 0) return SCMP_CMP_LT; - if (strcmp (name, "SCMP_CMP_LE") == 0) + if (strcmp (p, "LE") == 0) return SCMP_CMP_LE; - if (strcmp (name, "SCMP_CMP_EQ") == 0) + if (strcmp (p, "EQ") == 0) return SCMP_CMP_EQ; - if (strcmp (name, "SCMP_CMP_GE") == 0) + if (strcmp (p, "GE") == 0) return SCMP_CMP_GE; - if (strcmp (name, "SCMP_CMP_GT") == 0) + if (strcmp (p, "GT") == 0) return SCMP_CMP_GT; - if (strcmp (name, "SCMP_CMP_MASKED_EQ") == 0) + if (strcmp (p, "MASKED_EQ") == 0) return SCMP_CMP_MASKED_EQ; + +fail: return _SCMP_CMP_MIN; // Error. } @@ -134,8 +144,8 @@ get_seccomp_operator (const char *name, enum scmp_compare *op, libcrun_error_t * return 0; } -static int -get_seccomp_action (const char *name, int errno_ret, uint32_t *action, libcrun_error_t *err) +static uint32_t +get_seccomp_action_raw (const char *name, int errno_ret) { const char *p; @@ -146,61 +156,45 @@ get_seccomp_action (const char *name, int errno_ret, uint32_t *action, libcrun_e p += 9; if (strcmp (p, "ALLOW") == 0) - { - *action = SCMP_ACT_ALLOW; - return 0; - } - else if (strcmp (p, "ERRNO") == 0) - { - *action = SCMP_ACT_ERRNO (errno_ret); - return 0; - } - else if (strcmp (p, "KILL") == 0) - { - *action = SCMP_ACT_KILL; - return 0; - } + return SCMP_ACT_ALLOW; + if (strcmp (p, "ERRNO") == 0) + return SCMP_ACT_ERRNO (errno_ret); + if (strcmp (p, "KILL") == 0) + return SCMP_ACT_KILL; # ifdef SCMP_ACT_LOG - else if (strcmp (p, "LOG") == 0) - { - *action = SCMP_ACT_LOG; - return 0; - } + if (strcmp (p, "LOG") == 0) + return SCMP_ACT_LOG; # endif - else if (strcmp (p, "TRAP") == 0) - { - *action = SCMP_ACT_TRAP; - return 0; - } - else if (strcmp (p, "TRACE") == 0) - { - *action = SCMP_ACT_TRACE (errno_ret); - return 0; - } + if (strcmp (p, "TRAP") == 0) + return SCMP_ACT_TRAP; + if (strcmp (p, "TRACE") == 0) + return SCMP_ACT_TRACE (errno_ret); # ifdef SCMP_ACT_KILL_PROCESS - else if (strcmp (p, "KILL_PROCESS") == 0) - { - *action = SCMP_ACT_KILL_PROCESS; - return 0; - } + if (strcmp (p, "KILL_PROCESS") == 0) + return SCMP_ACT_KILL_PROCESS; # endif # ifdef SCMP_ACT_KILL_THREAD - else if (strcmp (p, "KILL_THREAD") == 0) - { - *action = SCMP_ACT_KILL_THREAD; - return 0; - } + if (strcmp (p, "KILL_THREAD") == 0) + return SCMP_ACT_KILL_THREAD; # endif # ifdef SCMP_ACT_NOTIFY - else if (strcmp (p, "NOTIFY") == 0) - { - *action = SCMP_ACT_NOTIFY; - return 0; - } + if (strcmp (p, "NOTIFY") == 0) + return SCMP_ACT_NOTIFY; # endif fail: - return crun_make_error (err, 0, "seccomp get action `%s`", name); + return ~0U; // Error. +} + +static int +get_seccomp_action (const char *name, int errno_ret, uint32_t *action, libcrun_error_t *err) +{ + *action = get_seccomp_action_raw (name, errno_ret); + + if (*action == ~0U) + return crun_make_error (err, 0, "seccomp get action `%s`", name); + + return 0; } #endif