Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755662AbbHYLwR (ORCPT ); Tue, 25 Aug 2015 07:52:17 -0400 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:56771 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755604AbbHYLwP (ORCPT ); Tue, 25 Aug 2015 07:52:15 -0400 X-Sasl-enc: aQMFItpd4MeleW9dp6W3NgQTDXnjw609Sf7EYkeVgvD0 1440503533 Date: Tue, 25 Aug 2015 13:52:11 +0200 From: Marek =?utf-8?Q?Marczykowski-G=C3=B3recki?= To: Luis Henriques Cc: Jiri Slaby , stable@vger.kernel.org, linux-kernel@vger.kernel.org, David Vrabel , Greg Kroah-Hartman Subject: Re: [PATCH 3.12 49/82] xen/gntdevt: Fix race condition in gntdev_release() Message-ID: <20150825115211.GB1345@mail-itl> References: <20150825113559.GD14096@ares> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="8GpibOaaTibBMecb" Content-Disposition: inline In-Reply-To: <20150825113559.GD14096@ares> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4770 Lines: 136 --8GpibOaaTibBMecb Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 25, 2015 at 12:35:59PM +0100, Luis Henriques wrote: > [ Adding Greg has he seems to have this patch queued for 3.10 and 3.14 ] >=20 > On Mon, Aug 24, 2015 at 11:09:09AM +0200, Jiri Slaby wrote: > > From: Marek Marczykowski-G=C3=B3recki > >=20 > > 3.12-stable review patch. If anyone has any objections, please let me = know. > >=20 > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > >=20 > > commit 30b03d05e07467b8c6ec683ea96b5bffcbcd3931 upstream. > >=20 > > While gntdev_release() is called the MMU notifier is still registered > > and can traverse priv->maps list even if no pages are mapped (which is > > the case -- gntdev_release() is called after all). But > > gntdev_release() will clear that list, so make sure that only one of > > those things happens at the same time. > >=20 > > Signed-off-by: Marek Marczykowski-G=C3=B3recki > > Signed-off-by: David Vrabel > > Signed-off-by: Jiri Slaby > > --- > > drivers/xen/gntdev.c | 2 ++ > > 1 file changed, 2 insertions(+) > >=20 > > diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c > > index e41c79c986ea..f2ca8d0af55f 100644 > > --- a/drivers/xen/gntdev.c > > +++ b/drivers/xen/gntdev.c > > @@ -529,12 +529,14 @@ static int gntdev_release(struct inode *inode, st= ruct file *flip) > > =20 > > pr_debug("priv %p\n", priv); > > =20 > > + mutex_lock(&priv->lock); >=20 > Since 3.12 doesn't seem to include 1401c00e59ea ("xen/gntdev: convert > priv->lock to a mutex"), this shouldn't be applied as priv->lock is > actually a spinlock. So, you'll need to pick 1401c00e59ea or backport > this patch using the appropriate locking directives. Not sure what's > the best solution. Maybe Marek or David can help...? I've used spinlock approach for some time (on 3.18.x) and it works ok. This= applies also to 3.10 and 3.14 of course. Patch here: https://raw.githubusercontent.com/QubesOS/qubes-linux-kernel/stable-3.18/pa= tches.xen/0001-xen-grant-fix-race-condition-in-gntdev_release.patch and here: =46rom b876e14888bdafa112c3265e6420543fa74aa709 Mon Sep 17 00:00:00 2001 =46rom: =3D?UTF-8?q?Marek=3D20Marczykowski-G=3DC3=3DB3recki?=3D Date: Fri, 26 Jun 2015 02:16:49 +0200 Subject: [PATCH] xen/grant: fix race condition in gntdev_release MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit Organization: Invisible Things Lab Cc: Marek Marczykowski-G=C3=B3recki While gntdev_release is called, MMU notifier is still registered and will traverse priv->maps list even if no pages are mapped (which is the case - gntdev_release is called after all). But gntdev_release will clear that list, so make sure that only one of those things happens at the same time. Signed-off-by: Marek Marczykowski-G=C3=B3recki --- drivers/xen/gntdev.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 8927485..4bd23bb 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -568,12 +568,14 @@ static int gntdev_release(struct inode *inode, struct= file *flip) =20 pr_debug("priv %p\n", priv); =20 + spin_lock(&priv->lock); while (!list_empty(&priv->maps)) { map =3D list_entry(priv->maps.next, struct grant_map, next); list_del(&map->next); gntdev_put_map(NULL /* already removed */, map); } WARN_ON(!list_empty(&priv->freeable_maps)); + spin_unlock(&priv->lock); =20 if (use_ptemod) mmu_notifier_unregister(&priv->mn, priv->mm); --=20 1.9.3 --=20 Best Regards, Marek Marczykowski-G=C3=B3recki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? --8GpibOaaTibBMecb Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBCAAGBQJV3FbrAAoJENuP0xzK19cs4/oH/3a6IUyZPNeVNj1665+qEMKK mRhwGiQcBw+Vu6St5t+3JqZUKp6A9wOB1B8azfkwsBMqCs8cleeHg4O9sOXuc4cy 0VjufMnjwjF2p9es6CmPXRu/gyKzoKMstL3KEHGb8SSVPwxCAV0GsNTXjRpJ3269 HItK4Y6Fl3iK+VunEVyFKy8OqNoLC6NmUdHGPfNuLuCXz1ltFkPSbiZnREc9Vwtc XjwX9DkFDqnEUAAH02u6pp3Vst3Ku5gD70n6o8Zav22WlnaVsU2C0zEnPeeYWTog LsivjBFmOi6Kz0z4Brl1sipl6hIOTfzFIlAMLLBJaLTbMU4ovRxIvLp2lVyKyZI= =lS5/ -----END PGP SIGNATURE----- --8GpibOaaTibBMecb-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/