The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/3562

This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.

=== Description (from pull-request) ===
Hello lxc team, 
I've added a few more fixes for the seccomp code. 
Please have a look at the logging output below.
I've added logging output for the current master without the changes (`before`) 
and for the current master including the changes  (`after`).


### skip compat archs when syscall is unavailable

I've also changed the loglevel from WARN to INFO if a syscall is undefined because this will flood the log 
in production with a lot of ephemeral containers (`crio-lxc`). E.g If kubernetes/cri-o  add a default seccomp profile
for kernel 5.n+1 but you're running kernel 5.n, there will likely be new syscalls that are undefined and flood logging.

before
```
lxc 20201027075832.580 INFO     seccomp - seccomp.c:parse_config_v2:795 - Processing "foobar allow"
lxc 20201027075832.580 WARN     seccomp - seccomp.c:do_resolve_add_rule:523 - Failed to resolve syscall "foobar"
lxc 20201027075832.580 WARN     seccomp - seccomp.c:do_resolve_add_rule:524 - This syscall will NOT be handled by seccomp
lxc 20201027075832.580 INFO     seccomp - seccomp.c:parse_config_v2:990 - Added native rule for arch 0 for foobar action 2147418112(allow)
lxc 20201027075832.580 WARN     seccomp - seccomp.c:do_resolve_add_rule:523 - Failed to resolve syscall "foobar"
lxc 20201027075832.580 WARN     seccomp - seccomp.c:do_resolve_add_rule:524 - This syscall will NOT be handled by seccomp
lxc 20201027075832.580 INFO     seccomp - seccomp.c:parse_config_v2:999 - Added compat rule for arch 1073741827 for foobar action 2147418112(allow)
lxc 20201027075832.580 WARN     seccomp - seccomp.c:do_resolve_add_rule:523 - Failed to resolve syscall "foobar"
lxc 20201027075832.580 WARN     seccomp - seccomp.c:do_resolve_add_rule:524 - This syscall will NOT be handled by seccomp
lxc 20201027075832.580 INFO     seccomp - seccomp.c:parse_config_v2:1009 - Added compat rule for arch 1073741886 for foobar action 2147418112(allow)
lxc 20201027075832.580 WARN     seccomp - seccomp.c:do_resolve_add_rule:523 - Failed to resolve syscall "foobar"
lxc 20201027075832.580 WARN     seccomp - seccomp.c:do_resolve_add_rule:524 - This syscall will NOT be handled by seccomp
lxc 20201027075832.580 INFO     seccomp - seccomp.c:parse_config_v2:1019 - Added native rule for arch -1073741762 for foobar action 2147418112(allow)
```

after
```
lxc 20201023145926.549 INFO     seccomp - seccomp.c:parse_config_v2:807 - Processing "foobar allow"
lxc 20201023145926.549 INFO     seccomp - seccomp.c:do_resolve_add_rule:530 - The syscall[foobar] is is undefined on host native arch
```

### fix handling of pseudo syscalls

before
```
lxc  20201023113416.552 INFO     seccomp - seccomp.c:parse_config_v2:796 - Processing "fadvise64_64 allow"
lxc 20201023113416.552 DEBUG    seccomp - seccomp.c:do_resolve_add_rule:531 - The syscall "fadvise64_64" is a pseudo syscall on hosts native arch.
lxc 20201023113416.552 INFO     seccomp - seccomp.c:parse_config_v2:991 - Added native rule for arch 0 for fadvise64_64 action 2147418112(allow)
lxc 20201023113416.552 INFO     seccomp - seccomp.c:parse_config_v2:1000 - Added compat rule for arch 1073741827 for fadvise64_64 action 2147418112(allow)
lxc 20201023113416.552 DEBUG    seccomp - seccomp.c:do_resolve_add_rule:536 - The syscall "fadvise64_64" nr:-10007 is not supported on compat arch:1073741886
lxc 20201023113416.552 INFO     seccomp - seccomp.c:parse_config_v2:1010 - Added compat rule for arch 1073741886 for fadvise64_64 action 2147418112(allow)
lxc 20201023113416.552 DEBUG    seccomp - seccomp.c:do_resolve_add_rule:536 - The syscall "fadvise64_64" nr:-10007 is not supported on compat arch:-1073741762
lxc 20201023113416.552 INFO     seccomp - seccomp.c:parse_config_v2:1020 - Added native rule for arch -1073741762 for fadvise64_64 action 2147418112(allow)
```

after
```
lxc 20201023141507.522 INFO     seccomp - seccomp.c:parse_config_v2:807 - Processing "fadvise64_64 allow"
lxc 20201023141507.522 DEBUG    seccomp - seccomp.c:do_resolve_add_rule:536 - The syscall[-10007:fadvise64_64] is a pseudo syscall and not available on host native arch.
lxc 20201023141507.522 INFO     seccomp - seccomp.c:do_resolve_add_rule:564 - Adding compat rule for syscall[-10007:fadvise64_64] action[2147418112:allow] arch[1073741827]
lxc 20201023141507.522 DEBUG    seccomp - seccomp.c:do_resolve_add_rule:541 - The syscall[-10007:fadvise64_64] is not supported on compat arch[1073741886]
```

### fixes invalid logging

before
```
lxc 20201023113416.551 INFO     seccomp - seccomp.c:parse_config_v2:796 - Processing "epoll_ctl_old allow"
lxc 20201023113416.551 INFO     seccomp - seccomp.c:parse_config_v2:991 - Added native rule for arch 0 for epoll_ctl_old action 2147418112(allow)
lxc 20201023113416.551 DEBUG    seccomp - seccomp.c:do_resolve_add_rule:536 - The syscall "epoll_ctl_old" nr:214 is not supported on compat arch:1073741827
lxc 20201023113416.551 INFO     seccomp - seccomp.c:parse_config_v2:1000 - Added compat rule for arch 1073741827 for epoll_ctl_old action 2147418112(allow)
lxc 20201023113416.551 DEBUG    seccomp - seccomp.c:do_resolve_add_rule:536 - The syscall "epoll_ctl_old" nr:214 is not supported on compat arch:1073741886
lxc 20201023113416.551 INFO     seccomp - seccomp.c:parse_config_v2:1010 - Added compat rule for arch 1073741886 for epoll_ctl_old action 2147418112(allow)
lxc  20201023113416.551 INFO     seccomp - seccomp.c:parse_config_v2:1020 - Added native rule for arch -1073741762 for epoll_ctl_old action 2147418112(allow)
```

after
```
lxc 20201023141642.835 INFO     seccomp - seccomp.c:parse_config_v2:807 - Processing "epoll_ctl_old allow"
lxc 20201023141642.835 INFO     seccomp - seccomp.c:do_resolve_add_rule:564 - Adding native rule for syscall[214:epoll_ctl_old] action[2147418112:allow] arch[0]
lxc 20201023141642.835 DEBUG    seccomp - seccomp.c:do_resolve_add_rule:541 - The syscall[214:epoll_ctl_old] is not supported on compat arch[1073741827]
lxc 20201023141642.835 DEBUG    seccomp - seccomp.c:do_resolve_add_rule:541 - The syscall[214:epoll_ctl_old] is not supported on compat arch[1073741886]
```
From 0ff0d23e4001ec9cadae51b41e834a954ef5026c Mon Sep 17 00:00:00 2001
From: Ruben Jenster <r.jens...@drachenfels.de>
Date: Thu, 22 Oct 2020 17:15:58 +0200
Subject: [PATCH 1/2] seccomp: Fix handling of pseudo syscalls and improve
 logging for rule processing.

Signed-off-by: Ruben Jenster <r.jens...@drachenfels.de>
---
 src/lxc/seccomp.c | 74 +++++++++++++++++++++++------------------------
 1 file changed, 36 insertions(+), 38 deletions(-)

diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c
index 03c1b54f01..f97e5cb86d 100644
--- a/src/lxc/seccomp.c
+++ b/src/lxc/seccomp.c
@@ -486,7 +486,14 @@ static scmp_filter_ctx get_new_ctx(enum lxc_hostarch_t 
n_arch, uint32_t default_
        return ctx;
 }
 
-static bool do_resolve_add_rule(uint32_t arch, char *line, scmp_filter_ctx ctx,
+enum lxc_seccomp_rule_status_t {
+  lxc_seccomp_rule_added = 0,
+  lxc_seccomp_rule_err,
+  lxc_seccomp_rule_undefined_syscall,
+  lxc_seccomp_rule_unsupported_arch,
+};
+
+static enum lxc_seccomp_rule_status_t do_resolve_add_rule(uint32_t arch, char 
*line, scmp_filter_ctx ctx,
                                struct seccomp_v2_rule *rule)
 {
        int i, nr, ret;
@@ -496,7 +503,7 @@ static bool do_resolve_add_rule(uint32_t arch, char *line, 
scmp_filter_ctx ctx,
        if (arch && ret != 0) {
                errno = -ret;
                SYSERROR("Seccomp: rule and context arch do not match (arch 
%d)", arch);
-               return false;
+               return lxc_seccomp_rule_err;
        }
 
        /*get the syscall name*/
@@ -511,29 +518,28 @@ static bool do_resolve_add_rule(uint32_t arch, char 
*line, scmp_filter_ctx ctx,
                if (ret < 0) {
                        errno = -ret;
                        SYSERROR("Failed loading rule to reject force umount");
-                       return false;
+                       return lxc_seccomp_rule_err;
                }
 
                INFO("Set seccomp rule to reject force umounts");
-               return true;
+               return lxc_seccomp_rule_added;
        }
 
        nr = seccomp_syscall_resolve_name(line);
        if (nr == __NR_SCMP_ERROR) {
-               WARN("Failed to resolve syscall \"%s\"", line);
-               WARN("This syscall will NOT be handled by seccomp");
-               return true;
+               INFO("The syscall[%s] is is undefined on host native arch", 
line);
+               return lxc_seccomp_rule_undefined_syscall;
        }
 
-       if (nr < 0) {
-               WARN("Got negative return value %d for syscall \"%s\"", nr, 
line);
-               WARN("This syscall will NOT be handled by seccomp");
-               return true;
+       // The syscall resolves to a pseudo syscall and may be available on 
compat archs.
+       if (nr < 0 && arch == SCMP_ARCH_NATIVE) {
+               DEBUG("The syscall[%d:%s] is a pseudo syscall and not available 
on host native arch.", nr, line);
+               return lxc_seccomp_rule_unsupported_arch;
        }
 
        if (arch != SCMP_ARCH_NATIVE && seccomp_syscall_resolve_name_arch(arch, 
line) < 0) {
-               INFO("The syscall \"%s\" nr:%d is not supported on compat 
arch:%d", line, nr, arch);
-               return true;
+               DEBUG("The syscall[%d:%s] is not supported on compat arch[%u]", 
nr, line, arch);
+               return lxc_seccomp_rule_unsupported_arch;
        }
 
        memset(&arg_cmp, 0, sizeof(arg_cmp));
@@ -555,16 +561,20 @@ static bool do_resolve_add_rule(uint32_t arch, char 
*line, scmp_filter_ctx ctx,
                                              rule->args_value[i].value);
        }
 
+       INFO("Adding %s rule for syscall[%d:%s] action[%d:%s] arch[%u]",
+                         (arch == SCMP_ARCH_NATIVE) ? "native" : "compat",
+                         nr, line, rule->action, 
get_action_name(rule->action), arch);
+
        ret = seccomp_rule_add_exact_array(ctx, rule->action, nr,
                                           rule->args_num, arg_cmp);
        if (ret < 0) {
                errno = -ret;
-               SYSERROR("Failed loading rule for %s (nr %d action %d (%s))",
-                        line, nr, rule->action, get_action_name(rule->action));
-               return false;
+               SYSERROR("Failed to add rule for syscall[%d:%s] action[%d:%s] 
arch[%u]",
+                        nr, line, rule->action, get_action_name(rule->action), 
arch);
+               return lxc_seccomp_rule_err;
        }
 
-       return true;
+       return lxc_seccomp_rule_added;
 }
 
 /*
@@ -983,42 +993,30 @@ static int parse_config_v2(FILE *f, char *line, size_t 
*line_bufsz, struct lxc_c
                }
 #endif
 
-               if (!do_resolve_add_rule(SCMP_ARCH_NATIVE, line,
-                                        conf->seccomp.seccomp_ctx, &rule))
-                       goto bad_rule;
 
-               INFO("Added native rule for arch %d for %s action %d(%s)",
-                    SCMP_ARCH_NATIVE, line, rule.action,
-                    get_action_name(rule.action));
+               ret = do_resolve_add_rule(SCMP_ARCH_NATIVE, line,
+                                        conf->seccomp.seccomp_ctx, &rule);
+               if (ret == lxc_seccomp_rule_err)
+                       goto bad_rule;
+               if (ret == lxc_seccomp_rule_undefined_syscall)
+                       continue;
 
                if (ctx.architectures[0] != SCMP_ARCH_NATIVE) {
-                       if (!do_resolve_add_rule(ctx.architectures[0], line,
+                       if (lxc_seccomp_rule_err == 
do_resolve_add_rule(ctx.architectures[0], line,
                                                 ctx.contexts[0], &rule))
                                goto bad_rule;
-
-                       INFO("Added compat rule for arch %d for %s action 
%d(%s)",
-                            ctx.architectures[0], line, rule.action,
-                            get_action_name(rule.action));
                }
 
                if (ctx.architectures[1] != SCMP_ARCH_NATIVE) {
-                       if (!do_resolve_add_rule(ctx.architectures[1], line,
+                       if (lxc_seccomp_rule_err == 
do_resolve_add_rule(ctx.architectures[1], line,
                                                 ctx.contexts[1], &rule))
                                goto bad_rule;
-
-                       INFO("Added compat rule for arch %d for %s action 
%d(%s)",
-                            ctx.architectures[1], line, rule.action,
-                            get_action_name(rule.action));
                }
 
                if (ctx.architectures[2] != SCMP_ARCH_NATIVE) {
-                       if (!do_resolve_add_rule(ctx.architectures[2], line,
+                       if (lxc_seccomp_rule_err == 
do_resolve_add_rule(ctx.architectures[2], line,
                                                ctx.contexts[2], &rule))
                                goto bad_rule;
-
-                       INFO("Added native rule for arch %d for %s action 
%d(%s)",
-                            ctx.architectures[2], line, rule.action,
-                            get_action_name(rule.action));
                }
        }
 

From 15044cd19c8454b20ee46fdb17dd0c8dd85366b1 Mon Sep 17 00:00:00 2001
From: Ruben Jenster <r.jens...@drachenfels.de>
Date: Fri, 23 Oct 2020 16:03:12 +0200
Subject: [PATCH 2/2] seccomp: Avoid duplicate processing of rules for host
 native arch.

Signed-off-by: Ruben Jenster <r.jens...@drachenfels.de>
---
 src/lxc/seccomp.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c
index f97e5cb86d..4faf693f6c 100644
--- a/src/lxc/seccomp.c
+++ b/src/lxc/seccomp.c
@@ -653,6 +653,8 @@ static int parse_config_v2(FILE *f, char *line, size_t 
*line_bufsz, struct lxc_c
                        default_rule_action = SCMP_ACT_ALLOW;
        }
 
+       DEBUG("Host native arch is [%u]", seccomp_arch_native());
+
        memset(&ctx, 0, sizeof(ctx));
        ctx.architectures[0] = SCMP_ARCH_NATIVE;
        ctx.architectures[1] = SCMP_ARCH_NATIVE;
@@ -1001,23 +1003,15 @@ static int parse_config_v2(FILE *f, char *line, size_t 
*line_bufsz, struct lxc_c
                if (ret == lxc_seccomp_rule_undefined_syscall)
                        continue;
 
-               if (ctx.architectures[0] != SCMP_ARCH_NATIVE) {
-                       if (lxc_seccomp_rule_err == 
do_resolve_add_rule(ctx.architectures[0], line,
-                                                ctx.contexts[0], &rule))
-                               goto bad_rule;
-               }
-
-               if (ctx.architectures[1] != SCMP_ARCH_NATIVE) {
-                       if (lxc_seccomp_rule_err == 
do_resolve_add_rule(ctx.architectures[1], line,
-                                                ctx.contexts[1], &rule))
-                               goto bad_rule;
+               for (int i = 0; i < 3; i++ ) {
+                       uint32_t arch = ctx.architectures[i];
+                       if (arch != SCMP_ARCH_NATIVE && arch != 
seccomp_arch_native()) {
+                               if (lxc_seccomp_rule_err == 
do_resolve_add_rule(arch, line,
+                                                       ctx.contexts[i], &rule))
+                                       goto bad_rule;
+                       }
                }
 
-               if (ctx.architectures[2] != SCMP_ARCH_NATIVE) {
-                       if (lxc_seccomp_rule_err == 
do_resolve_add_rule(ctx.architectures[2], line,
-                                               ctx.contexts[2], &rule))
-                               goto bad_rule;
-               }
        }
 
        INFO("Merging compat seccomp contexts into main context");
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to