virBitmapShrink: Do not attempt to clear bits beyond end of buffer

'virBitmapShrink' clears the bits beyond the end of the bitmap when
shrinking and then reallocates to match the new size. As it uses the
address of the first bit beyond the bitmap to do the clearing it can
overrun the allocated buffer if we're not actually going to shrink it
and the last bit's address is on the chunk boundary.

Fix it by returning in that corner case and add few more tests to be
sure.

Closes: https://gitlab.com/libvirt/libvirt/-/issues/673
Fixes: d6e582da80d
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 775bbf1532741001bc9c7079bc0ee2807960257b..b8d0352bb1216d9cc5425d76ba393f854dbdb1eb 100644 (file)
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -1183,6 +1183,12 @@ virBitmapShrink(virBitmap *map,
 
     nl = map->nbits / VIR_BITMAP_BITS_PER_UNIT;
     nb = map->nbits % VIR_BITMAP_BITS_PER_UNIT;
+
+    /* If we're at the end of the allocation the attempt to clear 'map->nbit'
+     * and further would be beyond the end of the bitmap */
+    if (nl >= map->map_alloc)
+        return;
+
     map->map[nl] &= ((1UL << nb) - 1);
 
     toremove = map->map_alloc - (nl + 1);

diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index adc956ca3da2a200e5fec0640cb29d8d0c95f136..27b6c13114540785330927efe01a6dbb19dffacf 100644 (file)
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -577,18 +577,45 @@ test12b(const void *opaque G_GNUC_UNUSED)
 {
     g_autoptr(virBitmap) map = NULL;
 
-    if (!(map = virBitmapParseUnlimited("34,1023")))
+    if (!(map = virBitmapParseUnlimited("31,32,63,64,1023")))
         return -1;
 
-    if (checkBitmap(map, "34,1023", 1024) < 0)
+    if (checkBitmap(map, "31-32,63-64,1023", 1024) < 0)
         return -1;
 
-    virBitmapShrink(map, 35);
-    if (checkBitmap(map, "34", 35) < 0)
+    /* no shrink at full alloc */
+    virBitmapShrink(map, 1025);
+    if (checkBitmap(map, "31-32,63-64,1023", 1024) < 0)
         return -1;
 
-    virBitmapShrink(map, 34);
-    if (checkBitmap(map, "", 34) < 0)
+    /* shrink at the end */
+    virBitmapShrink(map, 1023);
+    if (checkBitmap(map, "31-32,63-64", 1023) < 0)
+        return -1;
+
+    /* extend back to see whether tail was cleared */
+    virBitmapSetBitExpand(map, 1022);
+    if (checkBitmap(map, "31-32,63-64,1022", 1023) < 0)
+        return -1;
+
+    virBitmapShrink(map, 64);
+    if (checkBitmap(map, "31-32,63", 64) < 0)
+        return -1;
+
+    virBitmapShrink(map, 65);
+    if (checkBitmap(map, "31-32,63", 64) < 0)
+        return -1;
+
+    virBitmapShrink(map, 63);
+    if (checkBitmap(map, "31-32", 63) < 0)
+        return -1;
+
+    virBitmapShrink(map, 32);
+    if (checkBitmap(map, "31", 32) < 0)
+        return -1;
+
+    virBitmapShrink(map, 31);
+    if (checkBitmap(map, "", 31) < 0)
         return -1;
 
     return 0;