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.revia...@fobos-nt.ru> > Reported-by: Denis Rastyogin <ger...@altlinux.org> > Signed-off-by: Kevin Wolf <kw...@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é <berra...@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 :|