From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Zhang Haoyu" Subject: [PATCH] qcow2: fix double-free of Qcow2DiscardRegion in qcow2_process_discards Date: Sat, 11 Oct 2014 15:14:25 +0800 Message-ID: <201410111514227991260@sangfor.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: "kvm" , "Kevin Wolf" , "Stefan Hajnoczi" To: "qemu-devel" Return-path: Received: from smtp.sanfor.com ([58.251.49.30]:50173 "EHLO mail.sangfor.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1751539AbaJKHOa (ORCPT ); Sat, 11 Oct 2014 03:14:30 -0400 Sender: kvm-owner@vger.kernel.org List-ID: In qcow2_update_snapshot_refcount -> qcow2_process_discards() -> bdrv_discard() may free the Qcow2DiscardRegion which is referenced by "next" pointer in qcow2_process_discards() now, in next iteration, d = next, so g_free(d) will double-free this Qcow2DiscardRegion. qcow2_snapshot_delete |- qcow2_update_snapshot_refcount |-- qcow2_process_discards |--- bdrv_discard |---- aio_poll |----- aio_dispatch |------ bdrv_co_io_em_complete |------- qemu_coroutine_enter(co->coroutine, NULL); <=== coroutine entry is bdrv_co_do_rw |--- g_free(d) <== free first Qcow2DiscardRegion is okay |--- d = next; <== this set is done in QTAILQ_FOREACH_SAFE() macro. |--- g_free(d); <== double-free will happen if during previous iteration, bdrv_discard had free this object. bdrv_co_do_rw |- bdrv_co_do_writev |-- bdrv_co_do_pwritev |--- bdrv_aligned_pwritev |---- qcow2_co_writev |----- qcow2_alloc_cluster_link_l2 |------ qcow2_free_any_clusters |------- qcow2_free_clusters |-------- update_refcount |--------- qcow2_process_discards |---------- g_free(d) <== In next iteration, this Qcow2DiscardRegion will be double-free. Signed-off-by: Zhang Haoyu Signed-off-by: Fu Xuewei --- block/qcow2-refcount.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 2bcaaf9..3b759a3 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -462,9 +462,9 @@ fail_block: void qcow2_process_discards(BlockDriverState *bs, int ret) { BDRVQcowState *s = bs->opaque; - Qcow2DiscardRegion *d, *next; + Qcow2DiscardRegion *d; - QTAILQ_FOREACH_SAFE(d, &s->discards, next, next) { + while ((d = QTAILQ_FIRST(&s->discards)) != NULL) { QTAILQ_REMOVE(&s->discards, d, next); /* Discard is optional, ignore the return value */ -- 1.7.12.4 From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34595) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xcqsx-0006pH-5O for qemu-devel@nongnu.org; Sat, 11 Oct 2014 03:14:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xcqsq-0002go-QM for qemu-devel@nongnu.org; Sat, 11 Oct 2014 03:14:39 -0400 Received: from [58.251.49.30] (port=38475 helo=mail.sangfor.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xcqsq-0002g9-55 for qemu-devel@nongnu.org; Sat, 11 Oct 2014 03:14:32 -0400 Date: Sat, 11 Oct 2014 15:14:25 +0800 From: "Zhang Haoyu" Message-ID: <201410111514227991260@sangfor.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] [PATCH] qcow2: fix double-free of Qcow2DiscardRegion in qcow2_process_discards List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel Cc: Kevin Wolf , Stefan Hajnoczi , kvm In qcow2_update_snapshot_refcount -> qcow2_process_discards() -> bdrv_discard() may free the Qcow2DiscardRegion which is referenced by "next" pointer in qcow2_process_discards() now, in next iteration, d = next, so g_free(d) will double-free this Qcow2DiscardRegion. qcow2_snapshot_delete |- qcow2_update_snapshot_refcount |-- qcow2_process_discards |--- bdrv_discard |---- aio_poll |----- aio_dispatch |------ bdrv_co_io_em_complete |------- qemu_coroutine_enter(co->coroutine, NULL); <=== coroutine entry is bdrv_co_do_rw |--- g_free(d) <== free first Qcow2DiscardRegion is okay |--- d = next; <== this set is done in QTAILQ_FOREACH_SAFE() macro. |--- g_free(d); <== double-free will happen if during previous iteration, bdrv_discard had free this object. bdrv_co_do_rw |- bdrv_co_do_writev |-- bdrv_co_do_pwritev |--- bdrv_aligned_pwritev |---- qcow2_co_writev |----- qcow2_alloc_cluster_link_l2 |------ qcow2_free_any_clusters |------- qcow2_free_clusters |-------- update_refcount |--------- qcow2_process_discards |---------- g_free(d) <== In next iteration, this Qcow2DiscardRegion will be double-free. Signed-off-by: Zhang Haoyu Signed-off-by: Fu Xuewei --- block/qcow2-refcount.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 2bcaaf9..3b759a3 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -462,9 +462,9 @@ fail_block: void qcow2_process_discards(BlockDriverState *bs, int ret) { BDRVQcowState *s = bs->opaque; - Qcow2DiscardRegion *d, *next; + Qcow2DiscardRegion *d; - QTAILQ_FOREACH_SAFE(d, &s->discards, next, next) { + while ((d = QTAILQ_FIRST(&s->discards)) != NULL) { QTAILQ_REMOVE(&s->discards, d, next); /* Discard is optional, ignore the return value */ -- 1.7.12.4