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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list