qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH] qcow2: Check the L1 table parameters from all i


From: Kevin Wolf
Subject: Re: [Qemu-block] [PATCH] qcow2: Check the L1 table parameters from all internal snapshots
Date: Fri, 9 Feb 2018 16:03:31 +0100
User-agent: Mutt/1.9.1 (2017-09-22)

Am 09.02.2018 um 14:04 hat Max Reitz geschrieben:
> On 2018-02-09 12:37, Alberto Garcia wrote:
> > The code that reads the qcow2 snapshot table takes the offset and size
> > of all snapshots' L1 table without doing any kind of checks.
> > 
> > Although qcow2_snapshot_load_tmp() does verify that the table size is
> > valid, the table offset is not checked at all. On top of that there
> > are several other code paths that deal with internal snapshots that
> > don't use that function or do any other check.
> > 
> > This patch verifies that all L1 tables are correctly aligned and the
> > size does not exceed the maximum allowed valued.
> > 
> > The check from qcow2_snapshot_load_tmp() is removed since it's now
> > useless (opening an image will fail before that).
> > 
> > Signed-off-by: Alberto Garcia <address@hidden>
> > ---
> >  block/qcow2-snapshot.c     | 14 ++++++++++----
> >  tests/qemu-iotests/080     | 10 +++++++++-
> >  tests/qemu-iotests/080.out | 10 ++++++++--
> >  3 files changed, 27 insertions(+), 7 deletions(-)
> 
> I don't like completely refusing to open a qcow2 image if one of the
> snapshots is invalid, without giving the user any way of fixing it.
> 
> With this patch, the final two images created in 080 cannot be opened at
> all (not even with qemu-img info).  Without it, you can, you just can't
> load the snapshots.  (Well, in one case.  In the other, you can even do
> that, but that's a bug, as you correctly claim.)
> 
> More importantly, though, without this patch, you can delete the
> snapshots.  qemu-img will complain a bit, and you'll have leaks
> afterwards, but that's nothing qemu-img check can't fix.  With this
> patch, you can't, because the image cannot be opened at all so it's
> basically gone for good (unless you get a hex editor).

How about we move the check to bdrv_open() as proposed, but make it
conditional so that it's skipped with BDRV_O_CHECK and then add a way to
fix the situation with qemu-img check -r?

Kevin

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]