[Top][All Lists]

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

Re: [Qemu-devel] [PATCH v2 for-2.0 26/47] qcow2: Don't rely on free_clus

From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH v2 for-2.0 26/47] qcow2: Don't rely on free_cluster_index in alloc_refcount_block() (CVE-2014-0147)
Date: Fri, 28 Mar 2014 23:51:36 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0

On 28.03.2014 18:06, Stefan Hajnoczi wrote:
From: Kevin Wolf <address@hidden>

free_cluster_index is only correct if update_refcount() was called from
an allocation function, and even there it's brittle because it's used to
protect unfinished allocations which still have a refcount of 0 - if it
moves in the wrong place, the unfinished allocation can be corrupted.

So not using it any more seems to be a good idea. Instead, use the
first requested cluster to do the calculations. Return -EAGAIN if
unfinished allocations could become invalid and let the caller restart
its search for some free clusters.

The context of creating a snapsnot is one situation where
update_refcount() is called outside of a cluster allocation. For this
case, the change fixes a buffer overflow if a cluster is referenced in
an L2 table that cannot be represented by an existing refcount block.
(new_table[refcount_table_index] was out of bounds)

Signed-off-by: Kevin Wolf <address@hidden>
  * Fill new refcount block with zeroes when creating image.

    In v1 a dangling refcount table entry was created.  When a qcow2 image is
    created on a block device containing previous data (non-zero), the
    dangling refcount table entry would be read!

    Failure scenario:
    $ qemu-img create -f qcow2 /dev/loop0 30G
    $ qemu-img create -f qcow2 /dev/loop0 30G
    Huh, first cluster in empty image is already in use?

  block/qcow2-refcount.c     | 72 ++++++++++++++++++++++++----------------------
  block/qcow2.c              | 11 +++----
  tests/qemu-iotests/044.out |  2 +-
  tests/qemu-iotests/080     | 11 +++++++
  tests/qemu-iotests/080.out |  7 +++++
  5 files changed, 62 insertions(+), 41 deletions(-)

Reviewed-by: Max Reitz <address@hidden>

reply via email to

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