[libvirt] [PATCH 02/26] remote_driver: Resolve Coverity RESOURCE_LEAK

[Eric Blake]

On 09/11/2014 02:38 AM, Peter Krempa wrote:
> On 09/05/14 00:26, John Ferlan wrote:
>> Since 98b9acf5aa02551dd37d0209339aba2e22e4004a
>>
>> This ends up being a false positive for two reasons...
>>
>> expected to be already allocated and thus is passed by value; whereas,
>> the call into remoteDomainGetJobStats() 'params' is passed by reference.
>> Thus if the VIR_ALLOC is done there is no way for it to be leaked for
>> callers that passed by value.
>>
>> path that handles 'nparams == 0 && params == NULL' on entry. Thus all

Careful, my virDomainBlockCopy API passes nparams==0 && params ==
(non-NULL 0-length array) from the RPC daemon/remote.c receiver into the
libvirtd side of the API call.  It tripped me up the first time I tried
to assert that nparams==0 implied params==NULL, and failed the test.

>> other callers have guaranteed that 'params' is non NULL. Of course
>> Coverity isn't wise enough to pick up on this, but nonetheless is
>> does point out something "future callers" for which future callers
>> need to be aware.
>>
>> Even though it is a false positive, it's probably a good idea to at
>> least make some sort of check (and to "trick" Coverity into believing
>> we know what we're doing).  The easiest/cheapest way was to check
>> the input 'limit' value.  For the remoteDomainGetJobStats() it is
>> passed as 0 indicating (perhaps) that the caller has done the
>> limits length checking already and that its caller can handle
>> allocating something that can be passed back to the caller.

> 
> This unfortunately breaks the remote driver impl of GetAllDomainStats.
> As it seems that the limit parameter isn't used for automatically
> allocated arrays and I expected that it is I'll need either fix the
> remote impl of the stats function or add support for checking the limit
> here. And I probably prefer option 2, fixing
> remoteDeserializeTypedParameters to use limit correctly even for
> auto-alloced typed parameters.

I haven't looked closely at the patch, but it is a tricky area of code.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140911/e31e4955/attachment-0001.sig>