[Qemu-devel] [PATCH v2] Add argument filters to the seccomp sandbox

[namnamc]

> The current intention of the seccomp filter in QEMU, is that /all/ existing
> QEMU features continue to work unchanged. So even if a flag is used in a
> seemingly uncommon code path, we still need to allow that in a seccomp
> filter.
It already doesn't work very well, e.g. with -chroot, it fails because chroot()
is not whitelisted, same with -runas because setgid() etc isn't whitelisted.
Maybe there should be an extra option for -sandbox, like "-sandbox experimental"
which does argument filtering, which of course may break something, and the old
behavior would do plain syscall filtering without caring about arguments, 
because
that's so much easier to guarantee to work, even if it provides little security.

We could also change the default behavior from SCMP_ACT_KILL (which kills the
entire thing as soon as a single violation occurs) to SCMP_ACT_ERRNO(EPERM), 
which
will just return EPERM for a syscall with a violation. The software will be much
more capable of handling a permission denied error without crashing. Although of
course that violates the principle of fast-fail.

> So we need to add DODUMP, DONTDUMP, UNMERGABLE and WILLNEED here. That
> is still stricter than the previous allow-everything rule, so a net
> win.
And MADV_INVALID too I assume? That was one of the others I got with grep.

> > +
> > +    /* shmget */
> > +    rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
> > +        SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
> > +        SCMP_A2(SCMP_CMP_EQ, IP_CREAT|0777));
> 
> I'm not familiar with semantics of these seccomp rules, but is this
> saying that the second arg must be exactly equal to IP_CREAT|0777 ?
> If the app passes IP_CREAT|0600, would that be permitted instead ?
> The latter is what I see gtk2 source code passing for mode.
Argument 2 must be exactly equal to IP_CREAT|0777, yes, otherwise Qemu dies with
SIGSYS. I did check the Qemu source and saw 0777 was harcoded. Does the 0600 
mask
in GTK2 ever get called in Qemu? Anyway I added the MADV flags and the 0600 mask
in the v2 patch.

Signed-off-by: Namsun Ch'o <address@hidden>
---
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index f9de0d3..a353ef9 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -14,6 +14,8 @@
  */
 #include <stdio.h>
 #include <seccomp.h>
+#include <linux/ipc.h>
+#include <asm-generic/mman-common.h>
 #include "sysemu/seccomp.h"
 
 struct QemuSeccompSyscall {
@@ -105,7 +107,6 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] 
= {
     { SCMP_SYS(rt_sigreturn), 245 },
     { SCMP_SYS(sync), 245 },
     { SCMP_SYS(pread64), 245 },
-    { SCMP_SYS(madvise), 245 },
     { SCMP_SYS(set_robust_list), 245 },
     { SCMP_SYS(lseek), 245 },
     { SCMP_SYS(pselect6), 245 },
@@ -224,11 +225,9 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] 
= {
     { SCMP_SYS(arch_prctl), 240 },
     { SCMP_SYS(mkdir), 240 },
     { SCMP_SYS(fchmod), 240 },
-    { SCMP_SYS(shmget), 240 },
     { SCMP_SYS(shmat), 240 },
     { SCMP_SYS(shmdt), 240 },
     { SCMP_SYS(timerfd_create), 240 },
-    { SCMP_SYS(shmctl), 240 },
     { SCMP_SYS(mlockall), 240 },
     { SCMP_SYS(mlock), 240 },
     { SCMP_SYS(munlock), 240 },
@@ -264,6 +263,60 @@ int seccomp_start(void)
         }
     }
 
+    /* madvise */
+    static const int madvise_flags[] = {
+        MADV_DODUMP,
+        MADV_DONTDUMP,
+        MADV_INVALID,
+        MADV_UNMERGEABLE,
+        MADV_WILLNEED,
+        MADV_DONTFORK,
+        MADV_DONTNEED,
+        MADV_HUGEPAGE,
+        MADV_MERGEABLE,
+    };
+    for (i = 0; i < ARRAY_SIZE(madvise_flags); i++) {
+        rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(madvise), 1,
+            SCMP_A2(SCMP_CMP_EQ, madvise_flags[i]));
+        if (rc < 0) {
+            goto seccomp_return;
+        }
+    }
+    rc = seccomp_syscall_priority(ctx, SCMP_SYS(madvise), 245);
+    if (rc < 0) {
+        goto seccomp_return;
+    }
+
+    /* shmget */
+    rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
+        SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
+        SCMP_A2(SCMP_CMP_EQ, IP_CREAT|0777));
+    if (rc < 0) {
+        goto seccomp_return;
+    }
+    rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
+        SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
+        SCMP_A2(SCMP_CMP_EQ, IP_CREAT|0600));
+    if (rc < 0) {
+        goto seccomp_return;
+    }
+    rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmget), 240);
+    if (rc < 0) {
+        goto seccomp_return;
+    }
+
+    /* shmctl */
+    rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmctl), 2,
+        SCMP_A1(SCMP_CMP_EQ, IPC_RMID),
+        SCMP_A2(SCMP_CMP_EQ, 0));
+    if (rc < 0) {
+        goto seccomp_return;
+    }
+    rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmctl), 240);
+    if (rc < 0) {
+        goto seccomp_return;
+    }
+
     rc = seccomp_load(ctx);
 
   seccomp_return: