[libvirt] [PATCH 3/3] virbitmaptest: Add test for out of bounds condition

[Peter Krempa]

On 08/19/13 13:00, John Ferlan wrote:
> On 08/16/2013 06:32 AM, Peter Krempa wrote:
>> Previous patch fixed an issue where when parsing a bitmap from the a
>> string the bounds of the bitmap weren't checked. That flaw resulted into
>> crashes. This test tests that case to avoid it in the future.
>> ---
>>  tests/virbitmaptest.c | 34 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
>>
>> diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
>> index 8cfd8b5..c56d6fa 100644
>> --- a/tests/virbitmaptest.c
>> +++ b/tests/virbitmaptest.c
>> @@ -464,6 +464,38 @@ cleanup:
>>      return ret;
>>  }
>>
> 
> (just getting back from PTO :-))
> 
> Coverity found 3 RESOURCE_LEAK issues - all related though...  Looks
> like you're missing a "virBitmapFree(bitmap);"

Right, unfortunately they are false positives :)

> 
>> +
>> +/* test out of bounds conditions on virBitmapParse */
>> +static int
>> +test9(const void *opaque ATTRIBUTE_UNUSED)
>> +{
>> +    int ret = -1;
>> +    virBitmapPtr bitmap;
>> +
>> +    if (virBitmapParse("100000000", 0, &bitmap, 20) != -1)
>> +        goto cleanup;
>> +
> 
> (1) Event alloc_arg: 	"virBitmapParse(char const *, char, virBitmapPtr
> *, size_t)" allocates memory that is stored into "bitmap". [details]

All of those should fail, and only on a broken test they actually leak
the pointer. Such situation shouldn't ever happen in upstream (famous
last words? :) ).

> 
> 
>> +    if (bitmap)
>> +        goto cleanup;
>> +
>> +    if (virBitmapParse("1-1000000000", 0, &bitmap, 20) != -1)
>> +        goto cleanup;
>> +
>> +    if (bitmap)
>> +        goto cleanup;
>> +
>> +    if (virBitmapParse("1-10^10000000000", 0, &bitmap, 20) != -1)
>> +        goto cleanup;
>> +
>> +    if (bitmap)
>> +        goto cleanup;
>> +
>> +    ret = 0;
>> +cleanup:
>> +    return ret;
>> +
> 
> 494  	cleanup:
> 
> (5) Event leaked_storage: 	Variable "bitmap" going out of scope leaks
> the storage it points to.
> Also see events: 	[alloc_arg]
> 

But hopefully coverity will be smart enough that if I add a
virBitmapFree() here it won't complain any more.

> 495  	    return ret;
> 
> John
>> +}
>> +
>>  static int
>>  mymain(void)
>>  {
>> @@ -485,6 +517,8 @@ mymain(void)
>>          ret = -1;
>>      if (virtTestRun("test8", 1, test8, NULL) < 0)
>>          ret = -1;
>> +    if (virtTestRun("test9", 1, test9, NULL) < 0)
>> +        ret = -1;
>>
>>      return ret;
>>  }
>>
> 

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130819/26ac4d1c/attachment-0001.sig>