Re: [RFC PATCH] block/vpc: Support probing of fixed-size VHD images

[Kevin Wolf <kwolf@redhat.com>]

Am 29.03.2021 um 09:25 hat Thomas Huth geschrieben:
> Fixed-size VHD images don't have a header, only a footer. To be able
> to still detect them right, support probing via the file name, too.
> 
> Without this change, images get detected as raw:
> 
> $ qemu-img create -f vpc -o subformat=fixed test.vhd 2G
> Formatting 'test.vhd', fmt=vpc size=2147483648 subformat=fixed
> $ qemu-img info test.vhd
> image: test.vhd
> file format: raw
> virtual size: 2 GiB (2147992064 bytes)
> disk size: 8 KiB
> 
> With this change:
> 
> $ qemu-img info test.vhd
> image: test.vhd
> file format: vpc
> virtual size: 2 GiB (2147991552 bytes)
> disk size: 8 KiB
> 
> Resolves: https://bugs.launchpad.net/qemu/+bug/1819182
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  I've marked the subject with RFC since I'm not quite sure whether this
>  is really a good idea... please let me know what you think about it...

There is precedence for using the file name, and it's convenient, so
when done carefully, I think this is okay.

The classic problem we have with probing is that a malicious guest on a
probed raw image could write a $format header into the image and be
probed as something else the next time. For headers, we prevent this in
the raw driver, for filename based probes we don't.

Of course, if you call your raw image .vhd and use probing, you're
almost explicitly asking for trouble.

What happens if you do it anyway and the guest writes a VHD footer? In
theory, we can know that it's a VHD_FIXED image, and fixed image footers
don't contain anything that would allow a guest more than destroying
itself. Other than carrying the additional metadata in the footer, they
behave the same as raw images anyway.

We have a bug in vpc_open(), though: It sets the local variable
disk_type correctly, but other functions use s->footer.type instead and
we never check that it actually matches the disk type we think we're
opening.

So I think we need to add this check (and we need to add it even if we
don't change probing), and then the change to vpc_probe() is probably
okay.

Kevin