[libvirt] [PATCH] libvirt.c: don't let a NULL "cpumaps" argument provoke a NULL-deref

[Jim Meyering]

Daniel P. Berrange wrote:

> On Mon, Dec 14, 2009 at 05:18:54PM +0100, Jim Meyering wrote:
>>
>>
>> >From 94923b161a9d066146271bb533b78ab7877e4501 Mon Sep 17 00:00:00 2001
>> From: Jim Meyering <meyering at redhat.com>
>> Date: Mon, 14 Dec 2009 17:17:53 +0100
>> Subject: [PATCH] libvirt.c: don't let a NULL "cpumaps" argument provoke a NULL-deref
>>
>> * src/libvirt.c (virDomainGetVcpus): Update spec to say that maplen
>> is ignored when "cpumaps" is NULL.
>> Set maplen to 0 in that case.
>> ---
>>  src/libvirt.c |    7 +++++++
>>  1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/libvirt.c b/src/libvirt.c
>> index 008e322..4325aa4 100644
>> --- a/src/libvirt.c
>> +++ b/src/libvirt.c
>> @@ -4753,6 +4753,7 @@ error:
>>   *      virDomainPinVcpu() API.
>>   * @maplen: number of bytes in one cpumap, from 1 up to size of CPU map in
>>   *	underlying virtualization system (Xen...).
>> + *	Ignored when cpumaps is NULL.
>>   *
>>   * Extract information about virtual CPUs of domain, store it in info array
>>   * and also in cpumaps if this pointer isn't NULL.
>> @@ -4776,6 +4777,12 @@ virDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo,
>>          virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__);
>>          goto error;
>>      }
>> +
>> +    /* Ensure that domainGetVcpus (aka remoteDomainGetVcpus) does not
>> +       try to memcpy anything into a NULL pointer.  */
>> +    if (cpumaps == NULL)
>> +        maplen = 0;
>> +
>>      if (cpumaps != NULL && maplen < 1) {
>>          virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__);
>>          goto error;
>> --
>
> I wonder if it might be better to return an error in that case. Passing
> a NULL cpumaps, and non-zero maplen seems like a real application bug
> we should complain about
>
>   if (cpumaps == NULL && maplen != 0)
>     ....error...

Either way is fine with me.
I was trying to preserve what looked like existing intent.

Let me know.