mirrors/qemu
0
0 Fork 0
mirror of https://gitlab.com/qemu-project/qemu.git synced 2026-10-28 11:54:41 +08:00
Code Activity

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

Browse Source 
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>
This commit is contained in:
Daniel P. Berrangé committed 2026-10-15 13:03:41 +01:00
1 parent a567ef8655
commit e9f2ecafb1
1 file changed
-9
Download Patch File Download Diff File Expand all files Collapse all files
system/qemu-seccomp.c
-9
View File
@@ -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),