[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/5] qcow2: Metadata overlap checks
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 2/5] qcow2: Metadata overlap checks |
Date: |
Tue, 27 Aug 2013 13:16:18 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 27.08.2013 um 13:06 hat Max Reitz geschrieben:
> Am 27.08.2013 12:17, schrieb Kevin Wolf:
> >Am 26.08.2013 um 15:04 hat Max Reitz geschrieben:
> >>Two new functions are added; the first one checks a given range in the
> >>image file for overlaps with metadata (main header, L1 tables, L2
> >>tables, refcount table and blocks).
> >>
> >>The second one should be used immediately before writing to the image
> >>file as it calls the first function and, upon collision, marks the
> >>image as corrupt and makes the BDS unusable, thereby preventing
> >>further access.
> >>
> >>Both functions take a bitmask argument specifying the structures which
> >>should be checked for overlaps, making it possible to also check
> >>metadata writes against colliding with other structures.
> >>
> >>Signed-off-by: Max Reitz <address@hidden>
> >>---
> >> block/qcow2-refcount.c | 142
> >> +++++++++++++++++++++++++++++++++++++++++++++++++
> >> block/qcow2.h | 28 ++++++++++
> >> 2 files changed, 170 insertions(+)
> >>+ int64_t offset, int64_t size)
> >>+{
> >>+ BDRVQcowState *s = bs->opaque;
> >>+ int i, j;
> >>+
> >>+ if (!size) {
> >>+ return 0;
> >>+ }
> >>+
> >>+ if (chk & QCOW2_OL_MAIN_HEADER) {
> >>+ if (offset < s->cluster_size) {
> >>+ return QCOW2_OL_MAIN_HEADER;
> >>+ }
> >>+ }
> >>+
> >>+ if ((chk & QCOW2_OL_ACTIVE_L1) && s->l1_size) {
> >>+ if (ranges_overlap(offset, size, s->l1_table_offset,
> >>+ s->l1_size * sizeof(uint64_t))) {
> >The size could be rounded up to the next cluster boundary (same thing
> >for other metadata types).
> Would this actually change anything?
Not sure. With correct images, it wouldn't, because both the old and the
new offset would be aligned to a cluster boundary, so if there is an
overlap, it would be at the start. But after all, we're dealing with
broken images here.
I don't have a strong opinion on it, take it as a suggestion that you
can implement for additional safety. But if you don't want to, I don't
think it's a horrible gap in the checks.
Kevin
[Qemu-devel] [PATCH 4/5] qcow2: Check allocations in qcow2_check, Max Reitz, 2013/08/26