[libvirt] [PATCH] Handle copying bitmaps to larger data buffers

[Cole Robinson]

On 4/25/19 5:42 AM, Pavel Hrdina wrote:
> On Mon, Apr 15, 2019 at 02:43:07PM +0000, Allen, John wrote:
>> If a bitmap of a shorter length than the data buffer is passed to
>> virBitmapToDataBuf, it will read off the end of the bitmap and copy junk
>> into the returned buffer. Add a check to only copy the length of the
>> bitmap to the buffer.
>>
>> The problem can be observed after setting a vcpu affinity using the vcpupin
>> command on a system with a large number of cores:
>>   # virsh vcpupin example_domain 0 0
>>   # virsh vcpupin example_domain 0
>>      VCPU   CPU Affinity
>>     ---------------------------
>>      0      0,192,197-198,202
>>
>> Signed-off-by: John Allen <john.allen at amd.com>
>> ---
> 
> Thanks for the patch, pushed now.
> 
>> diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
>> index d074f29e54..021174cbb3 100644
>> --- a/src/util/virbitmap.c
>> +++ b/src/util/virbitmap.c
>> @@ -824,11 +824,15 @@ virBitmapToDataBuf(virBitmapPtr bitmap,
>>                     unsigned char *bytes,
>>                     size_t len)
>>  {
>> +    size_t nbytes = bitmap->map_len * (VIR_BITMAP_BITS_PER_UNIT / CHAR_BIT);
>>      unsigned long *l;
>>      size_t i, j;
>>  
>>      memset(bytes, 0, len);
>>  
>> +    /* If bitmap and buffer differ in size, only fill to the smaller length */
>> +    len = MIN(len, nbytes);
>> +
> 
> I would probably store the min back to nbytes and used that but it
> doesn't matter.
> 
>>      /* htole64 is not provided by gnulib, so we do the conversion by hand */
>>      l = bitmap->map;
>>      for (i = j = 0; i < len; i++, j++) {
> 

ACK to the patch as well. I hit a similar issue in my attempts to test
this, thought they had a different root cause but I was wrong, so ignore
my message attached to the test driver patch.

FWIW if someone wants to manually reproduce, grab my test driver patch
posted earlier and attached, and the testdriver.xml attached. Revert
this patch, build and reproduce with:

$ ./tools/virsh --connect test://`pwd`/testdriver.xml "vcpupin test 0 4;
vcpuinfo test"

Similarly this gives wrong results too, only first 13 host cpus should
be online:

$ ./tools/virsh --connect test://`pwd`/testdriver.xml nodecpumap

The latter is due to some bad virBitmapToData usage, but it's separate.

Thanks,
Cole
-------------- next part --------------
A non-text attachment was scrubbed...
Name: testdriver.xml
Type: text/xml
Size: 631 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190425/a23ce937/attachment-0001.xml>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-test-Use-nodeInfo-content-for-virNodeGetCPUMap.patch
Type: text/x-patch
Size: 2695 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190425/a23ce937/attachment-0001.bin>