From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53166) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afPM9-0005W0-Fx for qemu-devel@nongnu.org; Mon, 14 Mar 2016 06:04:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1afPM6-0002b7-5L for qemu-devel@nongnu.org; Mon, 14 Mar 2016 06:04:09 -0400 Received: from e06smtp14.uk.ibm.com ([195.75.94.110]:50161) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afPM5-0002ah-Si for qemu-devel@nongnu.org; Mon, 14 Mar 2016 06:04:06 -0400 Received: from localhost by e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 14 Mar 2016 10:04:02 -0000 References: <20160311184514.2768.97728.stgit@bahia.huguette.org> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= Message-ID: <56E68C89.30602@fr.ibm.com> Date: Mon, 14 Mar 2016 11:03:53 +0100 MIME-Version: 1.0 In-Reply-To: <20160311184514.2768.97728.stgit@bahia.huguette.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] spapr_rng: fix race with main loop List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz , David Gibson Cc: Thomas Huth , qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 03/11/2016 07:48 PM, Greg Kurz wrote: > Since commit "60253ed1e6ec rng: add request queue support to rng-random", > the use of a spapr_rng device may hang vCPU threads. > > The following path is taken without holding the lock to the main loop mutex: > > h_random() > rng_backend_request_entropy() > rng_random_request_entropy() > qemu_set_fd_handler() > > The consequence is that entropy_available() may be called before the vCPU > thread could even queue the request: depending on the scheduling, it may > happen that entropy_available() does not call random_recv()->qemu_sem_post(). > The vCPU thread will then sleep forever in h_random()->qemu_sem_wait(). > > This could not happen before 60253ed1e6ec because entropy_available() used > to call random_recv() unconditionally. > > This patch ensures the lock is held to avoid the race. > > Signed-off-by: Greg Kurz Reviewed-by: Cédric Le Goater Thanks for finding this race, C. > --- > > Thomas, > > This is the problem mentioned by Cedric in: > > https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg02526.html > > Cheers. > > -- > Greg > > hw/ppc/spapr_rng.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c > index a39d472b66fd..02d6be49f58e 100644 > --- a/hw/ppc/spapr_rng.c > +++ b/hw/ppc/spapr_rng.c > @@ -77,13 +77,13 @@ static target_ulong h_random(PowerPCCPU *cpu, sPAPRMachineState *spapr, > hrdata.val.v64 = 0; > hrdata.received = 0; > > - qemu_mutex_unlock_iothread(); > while (hrdata.received < 8) { > rng_backend_request_entropy(rngstate->backend, 8 - hrdata.received, > random_recv, &hrdata); > + qemu_mutex_unlock_iothread(); > qemu_sem_wait(&hrdata.sem); > + qemu_mutex_lock_iothread(); > } > - qemu_mutex_lock_iothread(); > > qemu_sem_destroy(&hrdata.sem); > args[0] = hrdata.val.v64; >