qcow2: fix null pointer dereference in crypto block

Commit Message

[e-mail] March 18, 2025, 10:58 a.m. UTC

From: Denis Rastyogin <[e-mail]>

This error was discovered by fuzzing qemu-img.

The qcow2_refresh_limits() is missing a check
for the s->crypto pointer, which can lead to
a null pointer dereference. This commit adds the necessary check.

Reported-by: Leonid Reviakin <[e-mail]>
Signed-off-by: Denis Rastyogin <[e-mail]>
---
 block/qcow2.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Kevin Wolf March 18, 2025, noon UTC | #1

Am 18.03.2025 um 11:58 hat [e-mail] geschrieben:
> From: Denis Rastyogin <[e-mail]>
> 
> This error was discovered by fuzzing qemu-img.
> 
> The qcow2_refresh_limits() is missing a check
> for the s->crypto pointer, which can lead to
> a null pointer dereference. This commit adds the necessary check.
> 
> Reported-by: Leonid Reviakin <[e-mail]>
> Signed-off-by: Denis Rastyogin <[e-mail]>

Please provide more information how to trigger this. This is probably
the wrong place to fix it. I think a qcow2 image that has bs->encrypted
should always also have s->crypto, so we need to understand why this
isn't the case here.

Kevin

[e-mail] March 18, 2025, 1 p.m. UTC | #2

You can reproduce this issue by running ./qemu-img info segv. 
The segv file used for reproduction can be found here: 
https://github.com/Gerben100/reproduce_qemu-img_error

Kevin Wolf March 18, 2025, 7:53 p.m. UTC | #3

Am 18.03.2025 um 14:00 hat [e-mail] geschrieben:
> You can reproduce this issue by running ./qemu-img info segv. 
> The segv file used for reproduction can be found here: 
> https://github.com/Gerben100/reproduce_qemu-img_error

Thanks. Your patch makes sure that qemu-img doesn't crash any more, but
we should catch the invalid image already while opening it. I'll send a
different patch.

Kevin

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index dd6bcafbd8..55861a285a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1978,6 +1978,10 @@  static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)
 
     if (bs->encrypted) {
         /* Encryption works on a sector granularity */
+        if (!s->crypto) {
+            error_setg(errp, "Encryption context is missing");
+            return;
+        }
         bs->bl.request_alignment = qcrypto_block_get_sector_size(s->crypto);
     }
     bs->bl.pwrite_zeroes_alignment = s->subcluster_size;