hbitmap: add assertion on hbitmap_iter_init

hbitmap_iter_init causes an out-of-bounds access when the "first"
argument is or greater than or equal to the size of the bitmap.
Forbid this with an assertion, and remove the failing testcase.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 7ddfb6680809903b989d549647c1e6238c595877..73f5d1d8d3e68386e14ead9411f635ee1127dfaf 100644 (file)
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -128,7 +128,8 @@ void hbitmap_free(HBitmap *hb);
  * hbitmap_iter_init:
  * @hbi: HBitmapIter to initialize.
  * @hb: HBitmap to iterate on.
- * @first: First bit to visit (0-based).
+ * @first: First bit to visit (0-based, must be strictly less than the
+ * size of the bitmap).
  *
  * Set up @hbi to iterate on the HBitmap @hb.  hbitmap_iter_next will return
  * the lowest-numbered bit that is set in @hb, starting at @first.

diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
index fcc6a00fc375a838c6c01c1d4ec89c37d110d524..8c902f2055fb45359d352f0cec183ac671e3550f 100644 (file)
--- a/tests/test-hbitmap.c
+++ b/tests/test-hbitmap.c
@@ -86,7 +86,9 @@ static void hbitmap_test_init(TestHBitmapData *data,
     data->bits = g_new0(unsigned long, n);
     data->size = size;
     data->granularity = granularity;
-    hbitmap_test_check(data, 0);
+    if (size) {
+        hbitmap_test_check(data, 0);
+    }
 }
 
 static void hbitmap_test_teardown(TestHBitmapData *data,
@@ -198,14 +200,6 @@ static void test_hbitmap_iter_partial(TestHBitmapData *data,
     hbitmap_test_check(data, L3 / 2);
 }
 
-static void test_hbitmap_iter_past(TestHBitmapData *data,
-                                    const void *unused)
-{
-    hbitmap_test_init(data, L3, 0);
-    hbitmap_test_set(data, 0, L3);
-    hbitmap_test_check(data, L3);
-}
-
 static void test_hbitmap_set_all(TestHBitmapData *data,
                                  const void *unused)
 {
@@ -388,7 +382,6 @@ int main(int argc, char **argv)
     hbitmap_test_add("/hbitmap/size/0", test_hbitmap_zero);
     hbitmap_test_add("/hbitmap/size/unaligned", test_hbitmap_unaligned);
     hbitmap_test_add("/hbitmap/iter/empty", test_hbitmap_iter_empty);
-    hbitmap_test_add("/hbitmap/iter/past", test_hbitmap_iter_past);
     hbitmap_test_add("/hbitmap/iter/partial", test_hbitmap_iter_partial);
     hbitmap_test_add("/hbitmap/iter/granularity", test_hbitmap_iter_granularity);
     hbitmap_test_add("/hbitmap/get/all", test_hbitmap_get_all);

diff --git a/util/hbitmap.c b/util/hbitmap.c
index fb7e01e8c5010503752897f3365c7304b9ac1f22..2aa487db74b0b817fd54911c31933c7ce3a4807e 100644 (file)
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -147,6 +147,7 @@ void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first)
 
     hbi->hb = hb;
     pos = first >> hb->granularity;
+    assert(pos < hb->size);
     hbi->pos = pos >> BITS_PER_LEVEL;
     hbi->granularity = hb->granularity;