All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Alberto Garcia <berto@igalia.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org,
	Max Reitz <mreitz@redhat.com>
Subject: Re: discard and v2 qcow2 images
Date: Mon, 23 Mar 2020 09:51:32 +0000	[thread overview]
Message-ID: <20200323095132.GB3379720@redhat.com> (raw)
In-Reply-To: <c0dcacfd-16cc-e2c2-304a-043e281d6bde@redhat.com>

On Fri, Mar 20, 2020 at 02:35:44PM -0500, Eric Blake wrote:
> On 3/20/20 1:58 PM, Alberto Garcia wrote:
> > Hi,
> > 
> > when full_discard is false in discard_in_l2_slice() then the selected
> > cluster should be deallocated and it should read back as zeroes. This
> > is done by clearing the cluster offset field and setting OFLAG_ZERO in
> > the L2 entry.
> > 
> > This flag is however only supported when qcow_version >= 3. In older
> > images the cluster is simply deallocated, exposing any possible
> > previous data from the backing file.
> 
> Discard is advisory, and has no requirements that discarded data read back
> as zero.  However, if write zeroes uses discard under the hood, then THAT
> usage must guarantee reading back as zero.
> 
> > 
> > This can be trivially reproduced like this:
> > 
> >     qemu-img create -f qcow2 backing.img 64k
> >     qemu-io -c 'write -P 0xff 0 64k' backing.img
> >     qemu-img create -f qcow2 -o compat=0.10 -b backing.img top.img
> >     qemu-io -c 'write -P 0x01 0 64k' top.img
> > 
> > After this, top.img is filled with 0x01. Now we issue a discard
> > command:
> > 
> >     qemu-io -c 'discard 0 64k' top.img
> > 
> > top.img should now read as zeroes, but instead you get the data from
> > the backing file (0xff). If top.img was created with compat=1.1
> > instead (the default) then it would read as zeroes after the discard.
> 
> I'd argue that this is undesirable behavior, but not a bug.

I think the ability to read old data from the backing file could
potentially be considered a security flaw, depending on what the
original data was in the backing file.

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 :|



      parent reply	other threads:[~2020-03-23  9:52 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-20 18:58 discard and v2 qcow2 images Alberto Garcia
2020-03-20 19:35 ` Eric Blake
2020-03-20 19:41   ` Alberto Garcia
2020-03-23  9:51   ` Daniel P. Berrangé [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200323095132.GB3379720@redhat.com \
    --to=berrange@redhat.com \
    --cc=berto@igalia.com \
    --cc=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.