From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58189) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WrmeC-0006KV-I3 for qemu-devel@nongnu.org; Tue, 03 Jun 2014 07:12:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wrme2-0005lh-Nw for qemu-devel@nongnu.org; Tue, 03 Jun 2014 07:12:52 -0400 Message-ID: <538DADA4.90502@redhat.com> Date: Tue, 03 Jun 2014 13:12:36 +0200 From: Paolo Bonzini 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; format=flowed 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: Ping Fan Liu , qemu-stable@nongnu.org, Stefan Priebe Il 03/06/2014 11:21, Stefan Hajnoczi ha scritto: > 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); > } > > > Reviewed-by: Paolo Bonzini