Re: [PATCH] openvz: convert popen to virCommand

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/04/2010 04:21 AM, Matthias Bolte wrote:
> 2010/12/3 Eric Blake <eblake@xxxxxxxxxx>:
>> popen must be matched with pclose (not fclose), or it will leak
>> resources.  Furthermore, it is a lousy interface when it comes
>> to signal handling.  We're much better off using our decent command
>> wrapper.
>>
>> +    line = *outbuf ? outbuf : NULL;
> 
> Is outbuf guaranteed to be non-NULL when virCommandRun succeeds?
> Otherwise we have a potential NULL dereference here.

Yep, I had already noticed that, which is why I posted two versions of a
fix to virCommand to cover alternative approaches to this problem (only
the v2 fix would guarantee non-NULL here).

>> +    if (virCommandRun(cmd, NULL) < 0) {
>> +        virCommandFree(cmd);
> 
> Is outbuf guaranteed to be unallocated when virCommandRun fails?
> Otherwise a VIR_FREE(outbuf) is missing here.

Not yet, but I think it should be (in other words, I need a v3 of my
virCommand fix that guarantees:

on success, if virCommandSet{Output,Error}Buffer was called, then that
buffer will be allocated to a non-NULL string, and caller is responsible
for freeing it

on failure, then those buffers are guaranteed to be NULL (if the caller
uses VIR_FREE, it won't hurt, but they do not have to explicitly worry
about partial data being collected on failure)

>> +    virCommandFree(cmd);
>> +    ok = sscanf(outbuf, "%d\n", &veid) == 1;
> 
> Same question here about outbuf being guaranteed to be non-NULL when
> virCommandRun succeeds as in openvzLoadDomains. If not then sscanf is
> called with NULL and that'll probably segfault.

That sscanf is overkill anyways; I can probably convert both *scanf
touched by this patch into lower-level functions, and in the process,
avoid problems such as ignoring overflow that are inherent in scanf.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]