1. Patchew
  2. QEMU
  3. View series

[PATCH] seccomp: drop redundant match for clone() flags == 0

Daniel P. Berrangé posted 1 patch 1 week, 3 days ago 
system/qemu-seccomp.c | 9 ---------
1 file changed, 9 deletions(-)
[PATCH] seccomp: drop redundant match for clone() flags == 0
Posted by Daniel P. Berrangé 1 week, 3 days ago 
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 psuedo 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
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 3889d48b4f2..8f8f8d3cf00 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
Re: [PATCH] seccomp: drop redundant match for clone() flags == 0
Posted by Marc-André Lureau 1 week, 2 days ago 
Hi

On Mon, Oct 12, 2026 at 6:25 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> 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 psuedo code of:

pseudo

other than that:
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>
>     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
> 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 3889d48b4f2..8f8f8d3cf00 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
>
>


-- 
Marc-André Lureau

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.