[PULL 2/8] seccomp: drop redundant match for clone() flags == 0

["Daniel P. Berrangé" <berrange@redhat.com>]

We have seccomp matches for clone() which mandate the presence of
several flags, in order to permit pthreads_create:

  REQUIRE_CLONE_FLAG(CLONE_VM);
  REQUIRE_CLONE_FLAG(CLONE_FS);
  ...

Given those mandatory matches, it is impossible for flags == 0 to
pass the filter. This is fortunate, since the explicit match on
flags == 0 is in fact broken, as it has a silent size extension
from 32-bits to 64-bits.

This results in seccomp generating pseudo code of:

    if ($arch == 3221225534)
      # filter for syscall "clone" (56) [priority: 65531]
      if ($syscall == 56)
        if ($a0.hi32 & 0xffffffff == 0)
          if ($a0.lo32 & 0xffffff00 == 0)
            action ERRNO(1);
      # default action
      action ALLOW;
    # invalid architecture action
    action KILL;

Since the clone() flag is passed as a 32-bit value in the syscall,
the high 32-bits are liable to be undefined/uninitialized garbage

The desired match for flags == 0 was

    if ($arch == 3221225534)
      # filter for syscall "clone" (56) [priority: 65532]
      if ($syscall == 56)
        if ($a0.hi32 & 0x00000000 == 0)
          if ($a0.lo32 & 0xffffff00 == 0)
            action ERRNO(1);
      # default action
      action ALLOW;
    # invalid architecture action
    action KILL;

That is redundant, however, as can be seen if we look at the
(fixed) program with the CLONE_THREAD rule also present:

    if ($arch == 3221225534)
      # filter for syscall "clone" (56) [priority: 65532]
      if ($syscall == 56)
        if ($a0.hi32 & 0x00000000 == 0)
          if ($a0.lo32 & 0xffffff00 == 0)
            action ERRNO(1);
          if ($a0.lo32 & 0x00010000 == 0)
            action ERRNO(1);
      # default action
      action ALLOW;
    # invalid architecture action
    action KILL;

That "($a0.lo32 & 0x00010000 == 0)" match will catch the case
of flags == 0 and do the right thing.

Reported-by: Ricardo Branco (https://gitlab.com/ricardobranco777)
Related: https://gitlab.com/qemu-project/qemu/-/work_items/4671
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 system/qemu-seccomp.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/system/qemu-seccomp.c b/system/qemu-seccomp.c
index cd2c1b0b747..a71cf66028d 100644
--- a/system/qemu-seccomp.c
+++ b/system/qemu-seccomp.c
@@ -76,13 +76,6 @@ const struct scmp_arg_cmp sched_setscheduler_arg[] = {
     { SCMP_SYS(clone),                  QEMU_SECCOMP_SET_SPAWN, \
       ARRAY_SIZE(clone_arg ## flag), clone_arg ## flag, SCMP_ACT_ERRNO(EPERM) }
 
-/* If no CLONE_* flags are set, except CSIGNAL, deny */
-const struct scmp_arg_cmp clone_arg_none[] = {
-    { .arg = CLONE_FLAGS_ARG,
-      .op = SCMP_CMP_MASKED_EQ,
-      .datum_a = ~(CSIGNAL), .datum_b = 0 }
-};
-
 /*
  * pthread_create should always set all of these.
  */
@@ -219,8 +212,6 @@ static const struct QemuSeccompSyscall denylist[] = {
       0, NULL, SCMP_ACT_ERRNO(EPERM) },
     { SCMP_SYS(execve),                 QEMU_SECCOMP_SET_SPAWN,
       0, NULL, SCMP_ACT_ERRNO(EPERM) },
-    { SCMP_SYS(clone),                  QEMU_SECCOMP_SET_SPAWN,
-      ARRAY_SIZE(clone_arg_none), clone_arg_none, SCMP_ACT_ERRNO(EPERM) },
     RULE_CLONE_FLAG(CLONE_VM),
     RULE_CLONE_FLAG(CLONE_FS),
     RULE_CLONE_FLAG(CLONE_FILES),
-- 
2.56.0