On Fri, Jul 20, 2018 at 05:44:24PM +0200, Marc-André Lureau wrote: > The seccomp action SCMP_ACT_KILL results in immediate termination of > the thread that made the bad system call. However, qemu being > multi-threaded, it keeps running. There is no easy way for parent > process / management layer (libvirt) to know about that situation. > > Instead, the default SIGSYS handler when invoked with SCMP_ACT_TRAP > will terminate the program and core dump. > > This may not be the most secure solution, but probably better than > just killing the offending thread. SCMP_ACT_KILL_PROCESS has been > added in Linux 4.14 to improve the situation, which I propose to use > by default if available in the next patch.
Note that seccomp doesn't promise to protect against all types of vulnerability in a program. It merely aims to stop the program executing designated system calls. Using SCMP_ACT_TRAP still prevents syscal execution to exactly the same extent that SCMP_ACT_KILL does, so its security level is the same. What differs is that the userspace app has option to ignore the syscall and carry on instead of being killed. A malicous attacker would thus have option to try to influence other parts of QEMU todo bad stuff, but if they already have control over the userspace process to this extent, they can likely do such bad stuff even before executing the syscalls So I don't think there's any significant difference in security protection here. Mostly the difference is just about what the crash will look like. A full process crash (from the default signal handler) looks better than a thread crash for the reasons you've explained. > > Related to: > https://bugzilla.redhat.com/show_bug.cgi?id=1594456 > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > --- > qemu-seccomp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/qemu-seccomp.c b/qemu-seccomp.c > index 9cd8eb9499..b117a92559 100644 > --- a/qemu-seccomp.c > +++ b/qemu-seccomp.c > @@ -125,7 +125,7 @@ static int seccomp_start(uint32_t seccomp_opts) > continue; > } > > - rc = seccomp_rule_add_array(ctx, SCMP_ACT_KILL, blacklist[i].num, > + rc = seccomp_rule_add_array(ctx, SCMP_ACT_TRAP, blacklist[i].num, > blacklist[i].narg, blacklist[i].arg_cmp); > if (rc < 0) { > goto seccomp_return; Reviewed-by: Daniel P. Berrangé <berra...@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
