Re: [PATCH] qcow2: Don't crash qemu-img info with missing crypto header

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Mar 18, 2025 at 09:11:43PM +0100, Kevin Wolf wrote:
> qcow2_refresh_limits() assumes that s->crypto is non-NULL whenever
> bs->encrypted is true. This is actually not the case: qcow2_do_open()
> allows to open an image with a missing crypto header for BDRV_O_NO_IO,
> and then bs->encrypted is true, but s->crypto is still NULL.
> 
> It doesn't make sense to open an invalid image, so remove the exception
> for BDRV_O_NO_IO. This catches the problem early and any code that makes
> the same assumption is safe now.
> 
> At the same time, in the name of defensive programming, we shouldn't
> make the assumption in the first place. Let qcow2_refresh_limits() check
> s->crypto rather than bs->encrypted. If s->crypto is NULL, it also can't
> make any requirement on request alignment.
> 
> Finally, start a qcow2-encryption test case that only serves as a
> regression test for this crash for now.
> 
> Reported-by: Leonid Reviakin <L.reviakin@fobos-nt.ru>
> Reported-by: Denis Rastyogin <gerben@altlinux.org>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.c                                 |  4 +-
>  tests/qemu-iotests/tests/qcow2-encryption     | 75 +++++++++++++++++++
>  tests/qemu-iotests/tests/qcow2-encryption.out | 32 ++++++++
>  3 files changed, 109 insertions(+), 2 deletions(-)
>  create mode 100755 tests/qemu-iotests/tests/qcow2-encryption
>  create mode 100644 tests/qemu-iotests/tests/qcow2-encryption.out

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|