From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40876) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XN172-0005FJ-Ea for qemu-devel@nongnu.org; Thu, 28 Aug 2014 10:55:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XN16u-00088Z-ML for qemu-devel@nongnu.org; Thu, 28 Aug 2014 10:55:44 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:58161 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XN16u-00088R-7K for qemu-devel@nongnu.org; Thu, 28 Aug 2014 10:55:36 -0400 Date: Thu, 28 Aug 2014 16:54:48 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140828145448.GD28789@irqsave.net> References: <1409213061-15562-1-git-send-email-rjones@redhat.com> <1409213061-15562-2-git-send-email-rjones@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1409213061-15562-2-git-send-email-rjones@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2] curl: Don't deref NULL pointer in call to aio_poll. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Richard W.M. Jones" Cc: pbonzini@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com The Thursday 28 Aug 2014 =E0 09:04:21 (+0100), Richard W.M. Jones wrote : > In commit 63f0f45f2e89b60ff8245fec81328ddfde42a303 the following > mechanical change was made: >=20 > if (!state) { > - qemu_aio_wait(); > + aio_poll(state->s->aio_context, true); > } >=20 > The new code now checks if state is NULL and then dereferences it > ('state->s') which is obviously incorrect. >=20 > This commit replaces state->s->aio_context with > bdrv_get_aio_context(bs), fixing this problem. The two other hunks > are concerned with getting the BlockDriverState pointer bs to where it > is needed. >=20 > The original bug causes a segfault when using libguestfs to access a > VMware vCenter Server and doing any kind of complex read-heavy > operations. With this commit the segfault goes away. >=20 > Signed-off-by: Richard W.M. Jones > Reviewed-by: Paolo Bonzini > --- > block/curl.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) >=20 > diff --git a/block/curl.c b/block/curl.c > index d4b85d2..f59615d 100644 > --- a/block/curl.c > +++ b/block/curl.c > @@ -352,7 +352,7 @@ static void curl_multi_timeout_do(void *arg) > #endif > } > =20 > -static CURLState *curl_init_state(BDRVCURLState *s) > +static CURLState *curl_init_state(BlockDriverState *bs, BDRVCURLState = *s) > { > CURLState *state =3D NULL; > int i, j; > @@ -370,7 +370,7 @@ static CURLState *curl_init_state(BDRVCURLState *s) > break; > } > if (!state) { > - aio_poll(state->s->aio_context, true); > + aio_poll(bdrv_get_aio_context(bs), true); > } > } while(!state); > =20 > @@ -541,7 +541,7 @@ static int curl_open(BlockDriverState *bs, QDict *o= ptions, int flags, > DPRINTF("CURL: Opening %s\n", file); > s->aio_context =3D bdrv_get_aio_context(bs); > s->url =3D g_strdup(file); > - state =3D curl_init_state(s); > + state =3D curl_init_state(bs, s); > if (!state) > goto out_noclean; > =20 > @@ -625,7 +625,7 @@ static void curl_readv_bh_cb(void *p) > } > =20 > // No cache found, so let's start a new request > - state =3D curl_init_state(s); > + state =3D curl_init_state(acb->common.bs, s); > if (!state) { > acb->common.cb(acb->common.opaque, -EIO); > qemu_aio_release(acb); > --=20 > 2.0.4 >=20 >=20 Reviewed-by: Beno=EEt Canet