Re: [Qemu-devel] [PATCH] seccomp: "-sandbox on" won't kill Qemu when option not built in

[Eduardo Otubo <otubo@linux.vnet.ibm.com>]

On 12/09/2013 03:33 PM, Daniel P. Berrange wrote:
> On Mon, Dec 09, 2013 at 03:20:52PM -0200, Eduardo Otubo wrote:
>> This option was requested by virt-test team so they can run tests with
>> Qemu and "-sandbox on" set without breaking whole test if host doesn't
>> have support for seccomp in kernel. It covers two possibilities:
>>
>>   1) Host kernel support does not support seccomp, but user installed Qemu
>>      package with sandbox support: Libseccomp will fail -> qemu will fail
>>      nicely and won't stop execution.
>>
>>   2) Host kernel has support but Qemu package wasn't built with sandbox
>>      feature. Qemu will fail nicely and won't stop execution.
>>
>> Signed-off-by: Eduardo Otubo <otubo@linux.vnet.ibm.com>
>> ---
>>   vl.c | 10 +++-------
>>   1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index b0399de..a0806dc 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -967,13 +967,11 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
>>   #ifdef CONFIG_SECCOMP
>>           if (seccomp_start() < 0) {
>>               qerror_report(ERROR_CLASS_GENERIC_ERROR,
>> -                          "failed to install seccomp syscall filter in the kernel");
>> -            return -1;
>> +                          "failed to install seccomp syscall filter in the kernel, disabling it");
>>           }
>>   #else
>>           qerror_report(ERROR_CLASS_GENERIC_ERROR,
>> -                      "sandboxing request but seccomp is not compiled into this build");
>> -        return -1;
>> +                      "sandboxing request but seccomp is not compiled into this build, disabling it");
>>   #endif
>>       }
>>
>> @@ -3808,9 +3806,7 @@ int main(int argc, char **argv, char **envp)
>>           exit(1);
>>       }
>>
>> -    if (qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL, 0)) {
>> -        exit(1);
>> -    }
>> +    qemu_opts_foreach(qemu_find_opts("sandbox"), parse_sandbox, NULL, 0);
>>
>>   #ifndef _WIN32
>>       if (qemu_opts_foreach(qemu_find_opts("add-fd"), parse_add_fd, NULL, 1)) {
>
> This change is really dubious from a security POV. If the admin requested
> sandboxing and the host or QEMU build cannot support it, then QEMU really
> *must* exit.

I think an admin must know what he's doing. If he requested sandbox but 
without kernel support he need to step back a little and understand what 
he's doing. This patch won't decrease the security level, IMHO.

>
> IMHO the test suite should probe to see if sandbox is working or not, and
> just not use the "-sandbox on" arg if the host doesn't support it.

But I think this could be done on virt-test as well :)

-- 
Eduardo Otubo
IBM Linux Technology Center