From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59528) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WroCv-000409-9z for qemu-devel@nongnu.org; Tue, 03 Jun 2014 08:52:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WroCp-0003ju-95 for qemu-devel@nongnu.org; Tue, 03 Jun 2014 08:52:49 -0400 Received: from mail-ph.de-nserver.de ([85.158.179.214]:60224) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WroCo-0003jd-VL for qemu-devel@nongnu.org; Tue, 03 Jun 2014 08:52:43 -0400 Message-ID: <538DC518.8080007@profihost.ag> Date: Tue, 03 Jun 2014 14:52:40 +0200 From: Stefan Priebe - Profihost AG MIME-Version: 1.0 References: <1401787261-30166-1-git-send-email-stefanha@redhat.com> In-Reply-To: <1401787261-30166-1-git-send-email-stefanha@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] aio: fix qemu_bh_schedule() bh->ctx race condition List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , qemu-devel@nongnu.org Cc: Paolo Bonzini , Ping Fan Liu , qemu-stable@nongnu.org Tested-by: Stefan Priebe Am 03.06.2014 11:21, schrieb 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 > --- > 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); > } > > >