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 <[email protected]>
Signed-off-by: Daniel P. Berrangé <[email protected]>
---
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
