Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755894AbbHYOJA (ORCPT ); Tue, 25 Aug 2015 10:09:00 -0400 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:51893 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755726AbbHYOI5 (ORCPT ); Tue, 25 Aug 2015 10:08:57 -0400 X-Sasl-enc: Z8vQNG65ZFsTksT/piU5T2IqHbfWpsO6pkXF26Df6o4P 1440511736 Date: Tue, 25 Aug 2015 16:08:53 +0200 From: Marek =?utf-8?Q?Marczykowski-G=C3=B3recki?= To: Jiri Slaby Cc: Luis Henriques , 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: <20150825140853.GC1345@mail-itl> References: <20150825113559.GD14096@ares> <20150825115211.GB1345@mail-itl> <55DC6B19.10001@suse.cz> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="0lnxQi9hkpPO77W3" Content-Disposition: inline In-Reply-To: <55DC6B19.10001@suse.cz> 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: 3997 Lines: 102 --0lnxQi9hkpPO77W3 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 25, 2015 at 03:18:17PM +0200, Jiri Slaby wrote: > On 08/25/2015, 01:52 PM, Marek Marczykowski-G=C3=B3recki wrote: > >>> --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ > >>> -529,12 +529,14 @@ static int gntdev_release(struct inode > >>> *inode, struct 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...? > >=20 > > 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. > >=20 > > Patch here:=20 > > https://raw.githubusercontent.com/QubesOS/qubes-linux-kernel/stable-3. > 18/patches.xen/0001-xen-grant-fix-race-condition-in-gntdev_release.patch > > > > and here: > >=20 > > From b876e14888bdafa112c3265e6420543fa74aa709 Mon Sep 17 00:00:00 > > 2001 From: =3D?UTF-8?q?Marek=3D20Marczykowski-G=3DC3=3DB3recki?=3D=20 > > 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 > > > >=20 > > 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. > >=20 > > Signed-off-by: Marek Marczykowski-G=C3=B3recki > > --- drivers/xen/gntdev.c | 2 ++ 1 > > file changed, 2 insertions(+) > >=20 > > 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);=20 > > list_del(&map->next); gntdev_put_map(NULL /* already removed */, > > map); } WARN_ON(!list_empty(&priv->freeable_maps)); + > > spin_unlock(&priv->lock); >=20 > Hmm, but e.g. > gntdev_put_map > -> gntdev_free_map > -> free_xenballooned_pages > -> mutex_lock >=20 > means sleep inside atomic, right? Indeed, you're probably right. But I haven't hit that problem ever... --=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? --0lnxQi9hkpPO77W3 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBCAAGBQJV3Hb1AAoJENuP0xzK19csVmsH/0+Pjp8FoV34nPPAfGaIqgw1 iCLnBWPW1EPu4+rMIpDgq3LERXKIldubkMrVpxJ79aSxZpdaMxkJjEb2147pl/wq JELg+JRECKKIAcgKrefHDpXzVUdoAMgjELPs7CkQ9xwP9GP1Hvyj6Yk+zenIkuTj CkpwJv8qFhb8qPNFoqtqTmqijXLkl7im7JW5MP6HgKQN4+VQsUWaE35eBlq5COf4 qPGQpt3gHWDiTfkOOGdYu61MOQo/6+v2rMZK9uEQ9UA2ytTqZ+hmwqU+Q8BldEzL GoYKA/6XaT1gvfkocoLuqM+3+YnB8QUVlawMhItUDx3SqcZTbARKlXFieiP2Bsk= =t/v/ -----END PGP SIGNATURE----- --0lnxQi9hkpPO77W3-- -- 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/