Re: [PATCH v3 4/4] qemu-img: Deprecate use of -b without -F

[Eric Blake]

On 3/9/20 10:31 AM, Kashyap Chamarthy wrote:

> After (with the patch series applied to QEMU Git):
>
>      $> git describe
>      v4.2.0-2204-gd6c7830114
>
>      # Create; *without* specifying "-F raw"
>      $> ~/build/qemu/qemu-img create -f qcow2 -b ./base.raw ./overlay2.qcow2
>      qemu-img: warning: Deprecated use of backing file without explicit backing 
> format (detected format of raw)
>      Formatting './overlay2.qcow2', fmt=qcow2 size=4294967296 
> backing_file=./base.raw backing_fmt=raw cluster_size=65536 lazy_refcounts=off 
> refcount_bits=16

If you'll note, this case _did_ write an implied backing_fmt=raw into 
the image.  Constrast that with creating an image on a qcow2 backing 
file, which tells you it detected a format of qcow2, but does NOT write 
backing_fmt=qcow2 into the image (this was a change from v2, at Peter's 
request).  Thus, when the backing is raw, we warn but future use of the 
image is now safe where it previously was not; when the backing file is 
non-raw, we warn but do not change our behavior, but because the backing 
file is non-raw any future probes will not be any less safe than before.

>      # Rebase; *without* specifying "-F raw"
>      $> ~/build/qemu/qemu-img rebase -b base.raw overlay1.qcow2
>      qemu-img: warning: Deprecated use of backing file without explicit backing 
> format, use of this image requires potentially unsafe format probing
>
>
> However, for the "Convert" case, is it correct that no warning is thrown
> for the below?
>
>      $> ~/build/qemu/qemu-img info overlay1.qcow2
>      image: overlay1.qcow2
>      file format: qcow2
>      virtual size: 4 GiB (4294967296 bytes)
>      disk size: 196 KiB
>      cluster_size: 65536
>      backing file: base.raw
>      Format specific information:
>          compat: 1.1
>          lazy refcounts: false
>          refcount bits: 16
>          corrupt: false

We have an image with no backing format, so we had to probe.  This patch 
series did not change the behavior of opening an existing image, only 
for creating a new image (or amending an image in-place).  So the lack 
of a warning on opening the unsafe image may be desirable, but it would 
be via even more patches.

>      
>      
>      $> ~/build/qemu/qemu-img convert -f qcow2 -O qcow2 overlay1.qcow2 flattened.raw

Ouch - you are creating a qcow2 destination file named 'flattened.raw', 
which is rather confusing on your part.

However, as your destination file is being created without a backing 
image, it is to be expected that there is no warning (when there is no 
backing file, -F makes no sense).  To provoke the warning during 
convert, you'll have to also pass -B (or -o backing_file), without -o 
backing_fmt (since convert lacks the -F shorthand).

>      $> echo $?
>      0
>
> > diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> > index 6c1d9034d9e3..a8ffacf54a52 100644
> > --- a/docs/system/deprecated.rst
> > +++ b/docs/system/deprecated.rst
> > @@ -376,6 +376,25 @@ The above, converted to the current supported format::
> >   Related binaries
> >   ----------------
> >
> > +qemu-img backing file without format (since 5.0.0)
> > +''''''''''''''''''''''''''''''''''''''''''''''''''
> > +
> > +The use of ``qemu-img create``, ``qemu-img rebase``, ``qemu-img
> > +convert``, or ``qemu-img amend`` to create or modify an image that
> > +depends on a backing file now recommends that an explicit backing
> > +format be provided.  This is for safety: if qemu probes a different
> > +format than what you thought, the data presented to the guest will be
> > +corrupt; similarly, presenting a raw image to a guest allows a
> > +potential security exploit if a future probe sees a non-raw image
> > +based on guest writes.  To avoid the warning message, or even future
> > +refusal to create an unsafe image, you must pass ``-o backing_fmt=``
> > +(or the shorthand ``-F`` during create) to specify the intended
> > +backing format.  You may use ``qemu-img rebase -u`` to retroactively
> > +add a backing format to an existing image.  However, be aware that
> > +there are already potential security risks to blindly using ``qemu-img
> > +info`` to probe the format of an untrusted backing image, when
> > +deciding what format to add into an existing image.
>
> Nit: s/qemu/QEMU/g/
>
> Ultra Nit: should this paragraph be broken down into two?  Experience
> tells people usually feel deterred read "substantial paragraphs" :-)

Could do, right before 'To avoid the warning'.

> I'll report back the Amend case.  (And once I get clarification on the
> Convert scenario, I'll be happy to give Tested-by.)
>
> [...]

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org