From 8e28f2c5c205c60752d22af16fc6e57a765fb61b Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Mon, 9 Sep 2024 16:46:03 +0200
Subject: [PATCH] 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>
---
 src/util/virbitmap.c  |  6 ++++++
 tests/virbitmaptest.c | 39 +++++++++++++++++++++++++++++++++------
 2 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 775bbf1532..b8d0352bb1 100644
--- 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 adc956ca3d..27b6c13114 100644
--- 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;