[libvirt] [PATCH] virpcitest: fix coverity issues

Pavel Hrdina phrdina at redhat.com
Thu Feb 6 16:36:05 UTC 2014


On 6.2.2014 16:48, Eric Blake wrote:
> On 02/06/2014 08:18 AM, Pavel Hrdina wrote:
>> The coverity server complains about the dev->stubDriver that it was
>> freed by the virPCIDeviceSetStubDriver function, but it somehow don't
>> get the fact that into the variable is immediately assigned new string.
>>
>> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
>> ---
>>   tests/virpcitest.c | 2 ++
>>   1 file changed, 2 insertions(+)
>
> What's the exact complaint?

The first one that coverity don't get that after free the new string is
assigned into the dev->stubDriver:

--------------------------------------------------------------------
Event freed_arg: "virPCIDeviceSetStubDriver" frees "dev->stubDriver".
--------------------------------------------------------------------

This is the root cause for another complaint that the dev->stubDriver is
dereferenced after free:

--------------------------------------------------------------------
Event deref_arg: Calling "virPCIDeviceDetach" dereferences freed
pointer "dev->stubDriver".
--------------------------------------------------------------------

>
>>
>> diff --git a/tests/virpcitest.c b/tests/virpcitest.c
>> index 994b300..8ff3b1d 100644
>> --- a/tests/virpcitest.c
>> +++ b/tests/virpcitest.c
>> @@ -248,6 +248,7 @@ testVirPCIDeviceDetachSingle(const void *opaque)
>>       if (!dev)
>>           goto cleanup;
>>
>> +    /* coverity[freed_arg] */
>>       if (virPCIDeviceSetStubDriver(dev, "pci-stub") < 0 ||
>>           virPCIDeviceDetach(dev, NULL, NULL) < 0)
>>           goto cleanup;
>
> Is this something where an sa_assert() could do the trick in a more
> tool-agnostic manner?
>

I'm looking at the code again and probably the issue found by coverity
could happen if the STRDUP fails inside of the
virPCIDeviceSetStubDriver.


This is a corner case where it could leads to creating a wrong modprobe
command because the dev->stubDriver also as driver and then as module
wouldn't be included in the command line.

--------------------------------------------------------------------
virPCIDeviceDetach()->virPCIProbeStubDriver()->virKModLoad()->
doModprobe()
--------------------------------------------------------------------

For the virKmodLoad() function the coverity found this issue:

--------------------------------------------------------------------
Event deref_parm_in_call:	Function "virKModLoad" dereferences
"driver". (The dereference is assumed on the basis of the 'nonnull'
parameter attribute.)
--------------------------------------------------------------------

I'm not sure what to do with this. At first I've thought that it's only
the coverity mistake.

Thanks for the review,

Pavel




More information about the libvir-list mailing list