On a Thursday in 2025, Peter Krempa via Devel wrote:
From: Peter Krempa <pkre...@redhat.com>

There are 3 bugs in the qcow2 header extension parser:

1) Header extension padding not taken into account

 Per qcow2 documentation (qemu.git/docs/interop/qcow2.txt, also now
 mirrored in the comment explaining the parser) each header extension
 entry is padded to a multiple of 8 bytes.

 Our parser didn't take the padding into account and advanced the
 current offset only by the 'length', which corresponds to the length
 of the data.

 This meant that in vast majority of cases only the first extension
 would be parsed correctly. For any other one we'd try to fetch the
 magic and length from wrong place.

 Luckily this wasn't problem for most of the almost 15 years this bug

a problem

 existed as we only cared about the backing file format string header
 which was always stored first by qemu.

 It is now a problem in the very specific case when a qcow2 image has a
 'data-file' and also a backing store with format. In such case we'd
 parse the backing store format properly as it's the first header and
 'data-file' being the second would be skipped.

 The buffer bounds checks were correct so we didn't violate any memory
 boundaries.

2) Integer underflow in calculation of end of header extension block

 If the image doesn't have a backing image, the 'backing_file_offset'
 qcow2 header field is 0. We use that value as 'extensions_end' which
 is used in the while loop to parse the extension entries.

 The check was done as "offset < (extensions_end - 8)", thus it
 unreflows when there's no filename.

 The design of the loop prevented anything bad from happening though.

3) Off-by-one when determining end of header extension block

 The aforementioned end of extension check above also has an off-by-one
 error as it allowed another loop if more than 8 bytes were available.

 Now the termination entry has data length of 0 bytes so we'd not be
 able to properly process that one.

 This wasn't a problem either as for now there's just the terminator
 having 0 byte lenght.

length


This patch improves documentation by quoting the qcow2 interoperability
document and adjusts the loop condition and lenght handling to comply
with the specs.

length


Interestingly we also had a test case for this specific scenario but the
expected test output was wrong.

Fixes: a93402d48b2996e5300754d299ef0d3f646aa098
Resolves: https://issues.redhat.com/browse/RHEL-93775
Signed-off-by: Peter Krempa <pkre...@redhat.com>
---
src/storage_file/storage_file_probe.c         | 62 +++++++++----------
.../out/qcow2datafile-qcow2_qcow2-datafile    | 10 ++-
2 files changed, 40 insertions(+), 32 deletions(-)


Reviewed-by: Ján Tomko <jto...@redhat.com>

Jano

Attachment: signature.asc
Description: PGP signature

Reply via email to