From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek =?utf-8?Q?Marczykowski-G=C3=B3recki?= Subject: Re: race condition in xen-gntdev Date: Fri, 26 Jun 2015 03:28:24 +0200 Message-ID: <20150626012824.GD967@mail-itl> References: <20150430144744.GF919@mail-itl> <20150527234508.GA14838@mail-itl> <20150617194211.GB11083@mail-itl> <20150622174626.GH5408@l.oracle.com> <20150622181335.GJ11083@mail-itl> <20150622183713.GD9631@l.oracle.com> <55885E88.2040805@tycho.nsa.gov> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2618176106913552753==" Return-path: In-Reply-To: <55885E88.2040805@tycho.nsa.gov> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Daniel De Graaf Cc: Boris Ostrovsky , xen-devel , David Vrabel List-Id: xen-devel@lists.xenproject.org --===============2618176106913552753== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="4ZLFUWh1odzi/v6L" Content-Disposition: inline --4ZLFUWh1odzi/v6L Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jun 22, 2015 at 03:14:16PM -0400, Daniel De Graaf wrote: > The reason that gntdev_release didn't have a lock is because there are not > supposed to be any references to the areas pointed to by priv->maps when = it > is called. However, since the MMU notifier has not yet been unregistered, > it is apparently possible to race here; the comment on mmu_notifier_unreg= ister > seems to confirm this as a possibility (as do the backtraces). >=20 > I think adding the lock will be sufficient. Ok, so here is the patch: -----------8<------------ =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 While gntdev_release is called, 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. 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 + mutex_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)); + mutex_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? --4ZLFUWh1odzi/v6L Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVjKq4AAoJENuP0xzK19csJFwH/2Z+TnvjW56gVe8LC+aTOWFQ l1cy/2cex3uLDTNQpBr5o2M1uArVehOR1FM0No2I082NPtV8EpBLhzMiFBU7iX+L S4Y1NGTP1mkcoLhRULgZ+I8Sms7WUTSrgLyePvkStDvweLBmLUfb1z2/EBAlAl8o lamy/DCfySuPcDpyl/whzztMXTEVwqo3NQmrdtCksdEEzhOlwBB3fu1phD1kh3Rb P8UZjKUXw0ZY1c6fsCBL6zOKffUzQgNNWjkzv6Um4rubTqeBCww4Iya/l3Uk/zxF sZR18cnX+yKomTqyUwzA7Xips8YeMQAw/T6YGFppYq8DQsIIkXiv8ECSUBUfqbI= =n0uS -----END PGP SIGNATURE----- --4ZLFUWh1odzi/v6L-- --===============2618176106913552753== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============2618176106913552753==--