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

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

[malike]

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")
Signed-off-by: malike <malike@kylinos.cn>
---
 block/bochs.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/block/bochs.c b/block/bochs.c
index bfda88017d..7a8091d47d 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -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))
     {
-- 
2.25.1

Re: [PATCH] block/bochs: validate bitmap size and compute extent_blocks after validation

[Kevin Wolf]

Am 14.10.2026 um 05:34 hat malike geschrieben:
> 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")
> Signed-off-by: malike <malike@kylinos.cn>

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