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

Re: [libvirt] [PATCH 01/11] tlscontext: Make sure to get proper pointer to name

On 01/31/2013 03:44 AM, Osier Yang wrote:
> On 2013年01月31日 03:36, John Ferlan wrote:
>> The 'dname' string was only filled in within the loop when available;
>> however, the TRACE macros used it unconditionally and caused Coverity
>> to compain about BAD_SIZEOF.  Using a dnameptr keeps Coverity at bay and


>> makes sure dname was properly filled before attempting the TRACE message.
>> ---
>>   src/rpc/virnettlscontext.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)

>> @@ -950,6 +950,7 @@ static int
>> virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt,
>>       unsigned int nCerts, i;
>>       char dname[256];
>>       size_t dnamesize = sizeof(dname);
>> +    char *dnameptr = NULL;

Would it be any simpler to just 0-initialize dname, as in:

char dname[256] = "";

>>             "ctxt=%p sess=%p dname=%s",
>> -          ctxt, sess, dname);

At which point, the PROBE(..., dname) would be guaranteed to have a NUL
terminator within range?  If I understand it, Coverity is complaining
that if dname is uninitialized, then the PROBE() may read beyond 256
bytes while looking for the end of a string.

> I guess dname[0] is guaranteed to be not nul as long as
> gnutls_x509_crt_get_dn succeeded.

Not unless we pre-initialize dname[0].

> If so, the patch can be simplified as:
> dname[0] ? dname : "(unknown)"

Using a conditional would make the difference between a probe stating
'dname=' vs. 'dname=(unknown)'; I don't think it adds that much to need
a ternary ?: in the PROBE.

Eric Blake   eblake redhat com    +1-919-301-3266
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]