Re: [Qemu-devel] [PATCHv3 3/3] seccomp: general fixes

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

On 09/11/2013 01:56 PM, Corey Bryant wrote:
>
>
> On 09/06/2013 03:21 PM, Eduardo Otubo wrote:
>>   1) On qemu-seccomp.c:255, the variable ctx was being used
>> uninitialized; now it's initialized with NULL and it's being checked at
>> the end of the function.
>>
>>   2) Changed the name of the command line option from "enable" to
>> "sandbox" for a better understanding from user side.
>>
>> Signed-off-by: Eduardo Otubo<otubo@linux.vnet.ibm.com>
>> ---
>>   qemu-seccomp.c | 5 +++--
>>   vl.c           | 6 +++---
>>   2 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
>> index 5e85eb5..f39d636 100644
>> --- a/qemu-seccomp.c
>> +++ b/qemu-seccomp.c
>> @@ -252,7 +252,7 @@ seccomp_return:
>>   int seccomp_start(int list_type)
>>   {
>>       int rc = 0;
>> -    scmp_filter_ctx ctx;
>> +    scmp_filter_ctx ctx = NULL;
>>
>>       switch (list_type) {
>>       case WHITELIST:
>> @@ -280,6 +280,7 @@ int seccomp_start(int list_type)
>>       rc = seccomp_load(ctx);
>>
>>   seccomp_return:
>> -    seccomp_release(ctx);
>> +    if (!ctx)
>
> You need to remove the ! from this check.
>
>> +        seccomp_release(ctx);
>>       return rc;
>>   }
>> diff --git a/vl.c b/vl.c
>> index 909f685..129919d 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -323,11 +323,11 @@ static QemuOptsList qemu_rtc_opts = {
>>
>>   static QemuOptsList qemu_sandbox_opts = {
>>       .name = "sandbox",
>> -    .implied_opt_name = "enable",
>> +    .implied_opt_name = "sandbox",
>
> So does this technically make it -sandbox,sandbox=on?If I understand

No. Qemu command line options is a little tricky and I had to spent some 
time to understand it. It actually make "-sandbox on,strict=on"

> correctly, I don't think the implied option is ever seen or used by the
> user anyway so it probably doesn't matter.  But I don't know if it's
> worth changing.

I changed the name so I can remember how it works in the future, since 
it's not that trivial.

>
>>       .head = QTAILQ_HEAD_INITIALIZER(qemu_sandbox_opts.head),
>>       .desc = {
>>           {
>> -            .name = "enable",
>> +            .name = "sandbox",
>>               .type = QEMU_OPT_BOOL,
>>           },{
>>               .name = "strict",
>> @@ -1036,7 +1036,7 @@ static int parse_sandbox(QemuOpts *opts, void
>> *opaque)
>>   {
>>       const char * strict_value = NULL;
>>       /* FIXME: change this to true for 1.3 */
>> -    if (qemu_opt_get_bool(opts, "enable", false)) {
>> +    if (qemu_opt_get_bool(opts, "sandbox", false)) {
>>   #ifdef CONFIG_SECCOMP
>>           if (seccomp_start(WHITELIST) < 0) {
>>               qerror_report(ERROR_CLASS_GENERIC_ERROR,
>> -- 1.8.3.1
>>
>

-- 
Eduardo Otubo
IBM Linux Technology Center