Re: [PULL 37/40] machine: add smp compound property

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Tue, 6 Jul 2021 at 11:39, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> Make -smp syntactic sugar for a compound property "-machine
>> smp.{cores,threads,cpu,...}".  machine_smp_parse is replaced by the
>> setter for the property.
>>
>> numa-test will now cover the new syntax, while other tests
>> still use -smp.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Hi; Coverity reports a leak in this code (CID 1458085):
>
>> +static void
>> +machine_parse_property_opt(QemuOptsList *opts_list, const char *propname,
>> +                           const char *arg, Error **errp)
>> +{
>> +    QDict *opts, *prop;
>> +    bool help = false;
>> +    ERRP_GUARD();
>> +
>> +    prop = keyval_parse(arg, opts_list->implied_opt_name, &help, errp);
>> +    if (help) {
>> +        qemu_opts_print_help(opts_list, true);
>> +        return;
>
> In this return path we don't unref 'prop'.
>
>> +    }

Moreover, when keyval_parse() fails without setting help, we pass an
errp that doesn't point to null to keyval_merge().  Potential crash
bug.

>> +    opts = qdict_new();
>> +    qdict_put(opts, propname, prop);
>> +    keyval_merge(machine_opts_dict, opts, errp);
>> +    qobject_unref(opts);
>> +}

I started to fix this, but got sidetracked into also fixing related
bugs.  Need to finish and and post.