Skip to contentSkip to conversation

block/bochs: validate bitmap size and compute extent_blocks after validation

PATCH Applied malike 2 participants 2 messages Oct 14 – Nov 10 Block layer

lore
Share & tools
lore

malike

author
#0 lore raw

PATCH 1/1 block/bochs: validate bitmap size and compute extent_blocks after validation

bochs_open() computes bitmap_blocks using the formula
"1 + (le32_to_cpu(bochs.bitmap) - 1) / 512". le32_to_cpu() returns uint32_t, so when bochs.bitmap is zero the subtraction wraps to UINT32_MAX (0xFFFFFFFF) in unsigned arithmetic, yielding bitmap_blocks = 8388608 instead of 1.

The bitmap field is read directly from the on-disk image header and was never validated. The overflowed bitmap_blocks propagates into seek_to_sector()'s bitmap_offset calculation, causing reads to target wrong offsets and return I/O errors or incorrect data.

Add an explicit zero check for bochs.bitmap before the formula.

Also move the extent_blocks calculation to after the existing extent_size validation. The same "1 + (x - 1) / 512" pattern applies to extent, but extent==0 is already caught by the "extent_size < BDRV_SECTOR_SIZE" check; the calculation is moved after that check for clarity and robustness, and uses the already-validated s->extent_size instead of re-reading the header field.

Fixes a8753c3466dc ("Bochs disk image support")
---
 block/bochs.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
block/bochs.c +8 -1
index bfda88017d..7a8091d47d 100644
@@ -167,8 +167,13 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
 
     s->data_offset = le32_to_cpu(bochs.header) + (s->catalog_size * 4);
 
+    if (le32_to_cpu(bochs.bitmap) == 0) {
+        error_setg(errp, "Bitmap size must not be zero");
+        ret = -EINVAL;
+        goto fail;
+    }
+
     s->bitmap_blocks = 1 + (le32_to_cpu(bochs.bitmap) - 1) / 512;
-    s->extent_blocks = 1 + (le32_to_cpu(bochs.extent) - 1) / 512;
 
     s->extent_size = le32_to_cpu(bochs.extent);
     if (s->extent_size < BDRV_SECTOR_SIZE) {
@@ -188,6 +193,8 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
+    s->extent_blocks = 1 + (s->extent_size - 1) / 512;
+
     if (s->catalog_size < DIV_ROUND_UP(bs->total_sectors,
                                        s->extent_size / BDRV_SECTOR_SIZE))
     {
Signature · 1 line
2.25.1
Applied
#1 lore raw

Am 14.10.2026 um 05:34 hat malike geschrieben:

Quoted 20 lines

bochs_open() computes bitmap_blocks using the formula
"1 + (le32_to_cpu(bochs.bitmap) - 1) / 512". le32_to_cpu() returns uint32_t, so when bochs.bitmap is zero the subtraction wraps to UINT32_MAX (0xFFFFFFFF) in unsigned arithmetic, yielding bitmap_blocks = 8388608 instead of 1.

The bitmap field is read directly from the on-disk image header and was never validated. The overflowed bitmap_blocks propagates into seek_to_sector()'s bitmap_offset calculation, causing reads to target wrong offsets and return I/O errors or incorrect data.

Add an explicit zero check for bochs.bitmap before the formula.

Also move the extent_blocks calculation to after the existing extent_size validation. The same "1 + (x - 1) / 512" pattern applies to extent, but extent==0 is already caught by the "extent_size < BDRV_SECTOR_SIZE" check; the calculation is moved after that check for clarity and robustness, and uses the already-validated s->extent_size instead of re-reading the header field.

Quoted 2 lines
Fixes a8753c3466dc ("Bochs disk image support")

Thanks, applied to the block branch.

Of course, this alone isn't enough to fully validate the bitmap size. Instead of incorrectly wrapping for a zero size bitmap and getting a too large number from this, the header field could directly specify an incorrect number that would have the same result in seek_to_sector().

qemu-iotests 078 could be extended to check different values for the bitmap size, too, to catch these cases.

Kevin