From patchwork Mon Mar 16 09:03:37 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fam Zheng X-Patchwork-Id: 450430 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 5F57B1400B6 for ; Mon, 16 Mar 2015 20:06:09 +1100 (AEDT) Received: from localhost ([::1]:47948 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YXQyM-0006eB-Pa for incoming@patchwork.ozlabs.org; Mon, 16 Mar 2015 05:06:06 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54418) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YXQwC-0003Gl-Bc for qemu-devel@nongnu.org; Mon, 16 Mar 2015 05:03:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YXQwB-0000PA-87 for qemu-devel@nongnu.org; Mon, 16 Mar 2015 05:03:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42505) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YXQwA-0000Oh-W5 for qemu-devel@nongnu.org; Mon, 16 Mar 2015 05:03:51 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t2G93nln009941 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Mon, 16 Mar 2015 05:03:50 -0400 Received: from ad.nay.redhat.com (dhcp-14-137.nay.redhat.com [10.66.14.137]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t2G93dXg013599; Mon, 16 Mar 2015 05:03:48 -0400 From: Fam Zheng To: qemu-devel@nongnu.org Date: Mon, 16 Mar 2015 17:03:37 +0800 Message-Id: <1426496617-10702-6-git-send-email-famz@redhat.com> In-Reply-To: <1426496617-10702-1-git-send-email-famz@redhat.com> References: <1426496617-10702-1-git-send-email-famz@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Paolo Bonzini Subject: [Qemu-devel] [PATCH v4 5/5] dma-helpers: Fix race condition of continue_after_map_failure and dma_aio_cancel X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org If DMA's owning thread cancels the IO while the bounce buffer's owning thread is notifying the "cpu client list", a use-after-free happens: continue_after_map_failure dma_aio_cancel ------------------------------------------------------------------ aio_bh_new qemu_bh_delete qemu_bh_schedule (use after free) Also, the old code doesn't run the bh in the right AioContext. Fix both problems by passing a QEMUBH to cpu_register_map_client. Signed-off-by: Fam Zheng Reviewed-by: Paolo Bonzini --- dma-helpers.c | 17 ++++++++--------- exec.c | 33 +++++++++++++++++++++------------ include/exec/cpu-common.h | 3 ++- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/dma-helpers.c b/dma-helpers.c index 6918572..1fddf6a 100644 --- a/dma-helpers.c +++ b/dma-helpers.c @@ -92,14 +92,6 @@ static void reschedule_dma(void *opaque) dma_blk_cb(dbs, 0); } -static void continue_after_map_failure(void *opaque) -{ - DMAAIOCB *dbs = (DMAAIOCB *)opaque; - - dbs->bh = qemu_bh_new(reschedule_dma, dbs); - qemu_bh_schedule(dbs->bh); -} - static void dma_blk_unmap(DMAAIOCB *dbs) { int i; @@ -161,7 +153,9 @@ static void dma_blk_cb(void *opaque, int ret) if (dbs->iov.size == 0) { trace_dma_map_wait(dbs); - cpu_register_map_client(dbs, continue_after_map_failure); + dbs->bh = aio_bh_new(blk_get_aio_context(dbs->blk), + reschedule_dma, dbs); + cpu_register_map_client(dbs->bh); return; } @@ -183,6 +177,11 @@ static void dma_aio_cancel(BlockAIOCB *acb) if (dbs->acb) { blk_aio_cancel_async(dbs->acb); } + if (dbs->bh) { + cpu_unregister_map_client(dbs->bh); + qemu_bh_delete(dbs->bh); + dbs->bh = NULL; + } } diff --git a/exec.c b/exec.c index 0fa7487..0f81358 100644 --- a/exec.c +++ b/exec.c @@ -2480,8 +2480,7 @@ typedef struct { static BounceBuffer bounce; typedef struct MapClient { - void *opaque; - void (*callback)(void *opaque); + QEMUBH *bh; QLIST_ENTRY(MapClient) link; } MapClient; @@ -2489,31 +2488,29 @@ QemuMutex map_client_list_lock; static QLIST_HEAD(map_client_list, MapClient) map_client_list = QLIST_HEAD_INITIALIZER(map_client_list); -static void cpu_unregister_map_client(void *_client); +static void cpu_unregister_map_client_do(MapClient *client); static void cpu_notify_map_clients_locked(void) { MapClient *client; while (!QLIST_EMPTY(&map_client_list)) { client = QLIST_FIRST(&map_client_list); - client->callback(client->opaque); - cpu_unregister_map_client(client); + qemu_bh_schedule(client->bh); + cpu_unregister_map_client_do(client); } } -void *cpu_register_map_client(void *opaque, void (*callback)(void *opaque)) +void cpu_register_map_client(QEMUBH *bh) { MapClient *client = g_malloc(sizeof(*client)); qemu_mutex_lock(&map_client_list_lock); - client->opaque = opaque; - client->callback = callback; + client->bh = bh; QLIST_INSERT_HEAD(&map_client_list, client, link); if (!atomic_read(&bounce.in_use)) { cpu_notify_map_clients_locked(); } qemu_mutex_unlock(&map_client_list_lock); - return client; } void cpu_exec_init_all(void) @@ -2526,14 +2523,26 @@ void cpu_exec_init_all(void) qemu_mutex_init(&map_client_list_lock); } -static void cpu_unregister_map_client(void *_client) +static void cpu_unregister_map_client_do(MapClient *client) { - MapClient *client = (MapClient *)_client; - QLIST_REMOVE(client, link); g_free(client); } +void cpu_unregister_map_client(QEMUBH *bh) +{ + MapClient *client; + + qemu_mutex_lock(&map_client_list_lock); + QLIST_FOREACH(client, &map_client_list, link) { + if (client->bh == bh) { + cpu_unregister_map_client_do(client); + break; + } + } + qemu_mutex_unlock(&map_client_list_lock); +} + static void cpu_notify_map_clients(void) { qemu_mutex_lock(&map_client_list_lock); diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h index fcc3162..43428bd 100644 --- a/include/exec/cpu-common.h +++ b/include/exec/cpu-common.h @@ -82,7 +82,8 @@ void *cpu_physical_memory_map(hwaddr addr, int is_write); void cpu_physical_memory_unmap(void *buffer, hwaddr len, int is_write, hwaddr access_len); -void *cpu_register_map_client(void *opaque, void (*callback)(void *opaque)); +void cpu_register_map_client(QEMUBH *bh); +void cpu_unregister_map_client(QEMUBH *bh); bool cpu_physical_memory_is_io(hwaddr phys_addr);