From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49260) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WswmZ-0003t9-ND for qemu-devel@nongnu.org; Fri, 06 Jun 2014 12:14:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WswmR-0004FY-Ru for qemu-devel@nongnu.org; Fri, 06 Jun 2014 12:14:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:22491) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WswmR-0004Ed-KF for qemu-devel@nongnu.org; Fri, 06 Jun 2014 12:14:11 -0400 From: Stefan Hajnoczi Date: Fri, 6 Jun 2014 18:13:22 +0200 Message-Id: <1402071243-16702-2-git-send-email-stefanha@redhat.com> In-Reply-To: <1402071243-16702-1-git-send-email-stefanha@redhat.com> References: <1402071243-16702-1-git-send-email-stefanha@redhat.com> Subject: [Qemu-devel] [PULL 01/42] aio: fix qemu_bh_schedule() bh->ctx race condition List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Peter Maydell , Stefan Hajnoczi qemu_bh_schedule() is supposed to be thread-safe at least the first time it is called. Unfortunately this is not quite true: bh->scheduled = 1; aio_notify(bh->ctx); Since another thread may run the BH callback once it has been scheduled, there is a race condition if the callback frees the BH before aio_notify(bh->ctx) has a chance to run. Reported-by: Stefan Priebe Signed-off-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Tested-by: Stefan Priebe --- async.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/async.c b/async.c index 6930185..5b6fe6b 100644 --- a/async.c +++ b/async.c @@ -117,15 +117,21 @@ void qemu_bh_schedule_idle(QEMUBH *bh) void qemu_bh_schedule(QEMUBH *bh) { + AioContext *ctx; + if (bh->scheduled) return; + ctx = bh->ctx; bh->idle = 0; - /* Make sure that idle & any writes needed by the callback are done - * before the locations are read in the aio_bh_poll. + /* Make sure that: + * 1. idle & any writes needed by the callback are done before the + * locations are read in the aio_bh_poll. + * 2. ctx is loaded before scheduled is set and the callback has a chance + * to execute. */ - smp_wmb(); + smp_mb(); bh->scheduled = 1; - aio_notify(bh->ctx); + aio_notify(ctx); } -- 1.9.3