From 42ed3377b5817f2c1f84e1bdca301ea51ecc3299 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>

Date: Thu, 20 Sep 2018 14:19:41 +0200

Subject: [PATCH] seccomp: tighten checking of seccomp filter creation

In seccomp code, the code is changed to propagate errors which are about

anything other than unknown/unimplemented syscalls. I *think* such errors

should not happen in normal usage, but so far we would summarilly ignore all

errors, so that part is uncertain. If it turns out that other errors occur and

should be ignored, this should be added later.

In nspawn, we would count the number of added filters, but didn't use this for

anything. Drop that part.

The comments suggested that seccomp_add_syscall_filter_item() returned negative

if the syscall is unknown, but this wasn't true: it returns 0.

The error at this point can only be if the syscall was known but couldn't be

added. If the error comes from our internal whitelist in nspawn, treat this as

error, because it means that our internal table is wrong. If the error comes

from user arguments, warn and ignore. (If some syscall is not known at current

architecture, it is still silently ignored.)

(cherry picked from commit 7e86bd73a47f2b8dd3d9a743e69fb0117f450ad8)

Related: #2040247

---

 src/nspawn/nspawn-seccomp.c | 14 +++++---------

 src/shared/seccomp-util.c   | 26 ++++++++++++++++----------

 2 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/src/nspawn/nspawn-seccomp.c b/src/nspawn/nspawn-seccomp.c

index fba22644da..17abfcec26 100644

--- a/src/nspawn/nspawn-seccomp.c

+++ b/src/nspawn/nspawn-seccomp.c

@@ -140,7 +140,7 @@ static int seccomp_add_default_syscall_filter(

                  */

         };

-        int r, c = 0;

+        int r;

         size_t i;

         char **p;

@@ -150,21 +150,17 @@ static int seccomp_add_default_syscall_filter(

                 r = seccomp_add_syscall_filter_item(ctx, whitelist[i].name, SCMP_ACT_ALLOW, syscall_blacklist, false);

                 if (r < 0)

-                        /* If the system call is not known on this architecture, then that's fine, let's ignore it */

-                        log_debug_errno(r, "Failed to add rule for system call %s on %s, ignoring: %m", whitelist[i].name, seccomp_arch_to_string(arch));

-                else

-                        c++;

+                        return log_error_errno(r, "Failed to add syscall filter item %s: %m", whitelist[i].name);

         }

         STRV_FOREACH(p, syscall_whitelist) {

                 r = seccomp_add_syscall_filter_item(ctx, *p, SCMP_ACT_ALLOW, syscall_blacklist, false);

                 if (r < 0)

-                        log_debug_errno(r, "Failed to add rule for system call %s on %s, ignoring: %m", *p, seccomp_arch_to_string(arch));

-                else

-                        c++;

+                        log_warning_errno(r, "Failed to add rule for system call %s on %s, ignoring: %m",

+                                          *p, seccomp_arch_to_string(arch));

         }

-        return c;

+        return 0;

 }

 int setup_seccomp(uint64_t cap_list_retain, char **syscall_whitelist, char **syscall_blacklist) {

diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c

index 4d2ba31d47..710a734715 100644

--- a/src/shared/seccomp-util.c

+++ b/src/shared/seccomp-util.c

@@ -907,9 +907,13 @@ int seccomp_add_syscall_filter_item(scmp_filter_ctx *seccomp, const char *name,

                 r = seccomp_rule_add_exact(seccomp, action, id, 0);

                 if (r < 0) {

                         /* If the system call is not known on this architecture, then that's fine, let's ignore it */

-                        if (log_missing)

-                                log_debug_errno(r, "Failed to add rule for system call %s() / %d, ignoring: %m",

-                                                name, id);

+                        bool ignore = r == -EDOM;

+

+                        if (!ignore || log_missing)

+                                log_debug_errno(r, "Failed to add rule for system call %s() / %d%s: %m",

+                                                name, id, ignore ? ", ignoring" : "");

+                        if (!ignore)

+                                return r;

                 }

                 return 0;

@@ -957,10 +961,8 @@ int seccomp_load_syscall_filter_set(uint32_t default_action, const SyscallFilter

                         return r;

                 r = seccomp_add_syscall_filter_set(seccomp, set, action, NULL, log_missing);

-                if (r < 0) {

-                        log_debug_errno(r, "Failed to add filter set, ignoring: %m");

-                        continue;

-                }

+                if (r < 0)

+                        return log_debug_errno(r, "Failed to add filter set: %m");

                 r = seccomp_load(seccomp);

                 if (IN_SET(r, -EPERM, -EACCES))

@@ -1005,11 +1007,15 @@ int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* set, u

                         if (r < 0) {

                                 /* If the system call is not known on this architecture, then that's fine, let's ignore it */

                                 _cleanup_free_ char *n = NULL;

+                                bool ignore;

                                 n = seccomp_syscall_resolve_num_arch(SCMP_ARCH_NATIVE, id);

-                                if (log_missing)

-                                        log_debug_errno(r, "Failed to add rule for system call %s() / %d, ignoring: %m",

-                                                        strna(n), id);

+                                ignore = r == -EDOM;

+                                if (!ignore || log_missing)

+                                        log_debug_errno(r, "Failed to add rule for system call %s() / %d%s: %m",

+                                                        strna(n), id, ignore ? ", ignoring" : "");

+                                if (!ignore)

+                                        return r;

                         }

                 }