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