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 <be...@igalia.com> > > --- > > 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
