7:09 a.m.
See patches for rationale.
Peter Krempa (4):
virBitmapNewCopy: Honor sizes of either bitmap when doing memcpy()
virbitmap: Extract and reuse buffer size calculation into a function
util: virbitmap: Extract clearing of unused bits at the end of the
last unit
util: bitmap: Rewrite virBitmapShrink using new helpers
src/util/virbitmap.c | 84 +++++++++++++++++++++-----------------------
1 file changed, 40 insertions(+), 44 deletions(-)
--
2.47.0
7:09 a.m.
New subject: [PATCH 1/4] virBitmapNewCopy: Honor sizes of either bitmap when doing memcpy()
'virBitmapNewCopy()' allocates a new bitmap with the same number of bits
but uses the internal allocation length as argument for the memcpy()
operation to copy the bits. Due to bugs in other code these may not be
the same resulting into a buffer overflow if the source is
over-allocated. Use the buffer length of the target bitmap instead.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virbitmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index b8d0352bb1..a1a8c5d126 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -582,7 +582,7 @@ virBitmapNewCopy(virBitmap *src)
{
virBitmap *dst = virBitmapNew(src->nbits);
- memcpy(dst->map, src->map, src->map_len * sizeof(src->map[0]));
+ memcpy(dst->map, src->map, dst->map_len * sizeof(src->map[0]));
return dst;
}
--
2.47.0
7:09 a.m.
New subject: [PATCH 2/4] virbitmap: Extract and reuse buffer size calculation into a function
Calculating the number of element can come handy in multiple places,
extract it from virBitmapNew.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virbitmap.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index a1a8c5d126..7dc63da6db 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -49,6 +49,22 @@ struct _virBitmap {
#define VIR_BITMAP_BIT(b) (1UL << VIR_BITMAP_BIT_OFFSET(b))
+/**
+ * Calculates and returns the number of elements in the bitmap buffer to fit @bits.
+ */
+static size_t
+virBitmapBuffsize(size_t nbits)
+{
+ if (SIZE_MAX - VIR_BITMAP_BITS_PER_UNIT < nbits) {
+ /* VIR_DIV_UP would overflow, let's overallocate by 1 entry instead of
+ * the potential overflow */
+ return (nbits / VIR_BITMAP_BITS_PER_UNIT) + 1;
+ }
+
+ return VIR_DIV_UP(nbits, VIR_BITMAP_BITS_PER_UNIT);
+}
+
+
/**
* virBitmapNew:
* @size: number of bits
@@ -61,15 +77,7 @@ virBitmap *
virBitmapNew(size_t size)
{
virBitmap *bitmap;
- size_t sz;
-
- if (SIZE_MAX - VIR_BITMAP_BITS_PER_UNIT < size) {
- /* VIR_DIV_UP would overflow, let's overallocate by 1 entry instead of
- * the potential overflow */
- sz = (size / VIR_BITMAP_BITS_PER_UNIT) + 1;
- } else {
- sz = VIR_DIV_UP(size, VIR_BITMAP_BITS_PER_UNIT);
- }
+ size_t sz = virBitmapBuffsize(size);
bitmap = g_new0(virBitmap, 1);
@@ -133,7 +141,7 @@ static void
virBitmapExpand(virBitmap *map,
size_t b)
{
- size_t new_len = VIR_DIV_UP(b + 1, VIR_BITMAP_BITS_PER_UNIT);
+ size_t new_len = virBitmapBuffsize(b + 1);
/* resize the memory if necessary */
if (map->map_len < new_len) {
--
2.47.0
7:09 a.m.
New subject: [PATCH 3/4] util: virbitmap: Extract clearing of unused bits at the end of the last unit
Extract the clearing of the traling bits from 'virBitmapSetAll' into a
new helper.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virbitmap.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 7dc63da6db..35cf729a22 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -757,6 +757,19 @@ virBitmapSize(virBitmap *bitmap)
}
+/**
+ * Internal helper that clears the unused bits at the end of the last bitmap unit.
+ */
+static void
+virBitmapClearTail(virBitmap *bitmap)
+{
+ size_t tail = bitmap->nbits % VIR_BITMAP_BITS_PER_UNIT;
+
+ if (tail)
+ bitmap->map[bitmap->map_len - 1] &= -1UL >>
(VIR_BITMAP_BITS_PER_UNIT - tail);
+}
+
+
/**
* virBitmapSetAll:
* @bitmap: the bitmap
@@ -765,15 +778,10 @@ virBitmapSize(virBitmap *bitmap)
*/
void virBitmapSetAll(virBitmap *bitmap)
{
- int tail = bitmap->nbits % VIR_BITMAP_BITS_PER_UNIT;
-
memset(bitmap->map, 0xff,
bitmap->map_len * (VIR_BITMAP_BITS_PER_UNIT / CHAR_BIT));
- /* Ensure tail bits are clear. */
- if (tail)
- bitmap->map[bitmap->map_len - 1] &=
- -1UL >> (VIR_BITMAP_BITS_PER_UNIT - tail);
+ virBitmapClearTail(bitmap);
}
--
2.47.0
7:09 a.m.
New subject: [PATCH 4/4] util: bitmap: Rewrite virBitmapShrink using new helpers
Rather than reimplement everything manually use virBitmapBuffsize to
find the current number of units, realloc the buffer and clear the tail
using virBitmapClearTail().
This fixes a corner case where the buffer would be over-allocated by one
unit when shrinking to the boundary of the unit size.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virbitmap.c | 34 +++++++---------------------------
1 file changed, 7 insertions(+), 27 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 35cf729a22..138c1ac5af 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -1187,33 +1187,13 @@ void
virBitmapShrink(virBitmap *map,
size_t b)
{
- size_t toremove;
- size_t nl = 0;
- size_t nb = 0;
-
- if (!map)
- return;
-
- if (map->nbits >= b)
- map->nbits = b;
-
- 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)
+ if (!map ||
+ map->nbits <= b)
return;
- map->map[nl] &= ((1UL << nb) - 1);
-
- toremove = map->map_alloc - (nl + 1);
-
- if (toremove == 0)
- return;
-
- VIR_SHRINK_N(map->map, map->map_alloc, toremove);
-
- /* length needs to be fixed as well */
- map->map_len = map->map_alloc;
+ map->nbits = b;
+ map->map_len = virBitmapBuffsize(b);
+ map->map = g_renew(unsigned long, map->map, map->map_len);
+ map->map_alloc = map->map_len;
+ virBitmapClearTail(map);
}
--
2.47.0
7:38 a.m.
New subject: [PATCH 0/4] virbitmap: Fix buffer overflow when shrinking a
bitmap and copying it
On Thu, Oct 17, 2024 at 14:09:08 +0200, Peter Krempa wrote:
See patches for rationale.
Peter Krempa (4):
virBitmapNewCopy: Honor sizes of either bitmap when doing memcpy()
virbitmap: Extract and reuse buffer size calculation into a function
util: virbitmap: Extract clearing of unused bits at the end of the
last unit
util: bitmap: Rewrite virBitmapShrink using new helpers
src/util/virbitmap.c | 84 +++++++++++++++++++++-----------------------
1 file changed, 40 insertions(+), 44 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
35
days inactive
35
days old
5 comments
2 participants
participants (2)
-
Jiri Denemark -
Peter Krempa