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

Re: [libvirt] [PATCH] network: plug unininitialized read found by valgrind



On 01/10/2011 06:54 PM, Laine Stump wrote:
> On 01/10/2011 05:28 PM, Eric Blake wrote:
>> * src/util/network.c (virSocketAddrMask): Zero out port, so that
>> iptables can initialize just the netmask, then call
>> virSocketFormatAddr without an uninitialized read in getnameinfo.
>> ---
>>
>> I'm not sure if this is the best patch; an alternative would be
>> to call memset(network,0,sizeof network) in iptablesFormatNetwork
>> prior to virSocketAddrMaskByPrefix.
> 
> (Sigh. I had visited this exact piece of the code due to an error during
> getnameinfo, and thought that setting network->len was the only thing
> missing.)

getnameinfo is documented as reading both addr and port, depending on
the flags.  Yes, this approach still leaves other fields in the
virSocketAddrPtr uninitialized, but valgrind didn't catch any invalid
uses of those other fields.

> I think this is the right place to fix it; A caller to
> virSocketAddrMaskByPrefix (or virSocketAddrMask) shouldn't need to
> initialize the virSocketAddr (I think it really should be an opaque type
> outside of network.c, although it isn't always treated that way, even by
> me).

And we can't blindly memset() inside virSocketAddrMask, since we
document (and at least one client uses) that the output addr argument
can be the same memory as the incoming argument being masked, so setting
used fields is the best we can do from within network.c to isolate the
clients from having to use memset() themselves.

> 
> ACK.

Thanks; pushed.

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

Attachment: signature.asc
Description: OpenPGP digital signature


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