[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
signature.asc
Description: PGP signature