On 20/07/2018 - 17:00:39, Daniel P. Berrange wrote: > 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.
I guess that's the whole point of having the process killed instead of the thread. Seccomp is not a big security feature alone by itself, but rather combined with others techniques. Marc, from what we've already discussed I think these patches are good enough for now. Thanks a lot for the contribution. > > > > > 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> > Acked-by: Eduardo Otubo <ot...@redhat.com>
signature.asc
Description: PGP signature