From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [linux-3.14 bisection] complete test-amd64-i386-xl-qcow2 Date: Fri, 9 Oct 2015 10:15:14 +0100 Message-ID: <1444382114.1410.304.camel@citrix.com> References: <1441099198.27618.13.camel@citrix.com> <21989.30329.419566.715627@mariner.uk.xensource.com> <1441101938.27618.17.camel@citrix.com> <1441185512.26292.111.camel@citrix.com> <1444342480.2956.274.camel@decadent.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <1444342480.2956.274.camel@decadent.org.uk> Sender: stable-owner@vger.kernel.org To: Ben Hutchings , Ian Jackson , stable@vger.kernel.org, Greg Kroah-Hartman Cc: osstest service owner , xen-devel@lists.xensource.com, Marek =?ISO-8859-1?Q?Marczykowski-G=F3recki?= , David Vrabel List-Id: xen-devel@lists.xenproject.org On Thu, 2015-10-08 at 23:14 +0100, Ben Hutchings wrote: > On Wed, 2015-09-02 at 10:18 +0100, Ian Campbell wrote: > > [resending to correct stable address, sorry folks] > >=20 > > TL;DR: Any backport of 30b03d05e074 to earlier than commit 1401c00e= 59e > > ("xen/gntdev: convert priv->lock to a mutex", which was added in v4= =2E0) > > needs $something doing to it, either s/mutex/spinlock/ or (more lik= ely) > > backporting of 1401c00e59e too. > >=20 > > Looking at LTS: > >=20 > > 3.18.y:> > Backported both. > > 3.16.y:> > Has backported neither > > 3.14.y:> > * Only backported 30b03d05e074 > > 3.12.y:> > Has backported neither > > 3.10.y:> > * Only backported 30b03d05e074 > > 3.4.y:> > Has backported neither > > 3.2.y:> > Has backported neither > >=20 > > So AFAICT 3.14.y and 3.10.y need fixes, probably following 3.18 and > > backporting 1401c00e59e. > >=20 > > 3.16/12/4/2 might need to be careful if they subsequently pick up > > 30b03d05. > [...] >=20 > I came up with the patch below for 3.2. Let me know if it's not > correct. =46WIW most of the other stable branches just took 1401c00e59e which is= now pretty well baked in mainline as well as various stable trees and saves having to reason about dropping the lock over gntdev_put_map. Was backporting that one to 3.2 problematic? I suppose it is safe to drop the lock because map is removed from the l= ist with the lock held, but I'm not 100% confident in that, and this gntdev stuff isn't really my bailiwick anyway so I'll back away now... Ian. >=20 > Ben. >=20 > --- > From: Marek Marczykowski-G=C3=B3recki > Date: Fri, 26 Jun 2015 03:28:24 +0200 > Subject: xen/gntdevt: Fix race condition in gntdev_release() >=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 i= s > 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 < > marmarek@invisiblethingslab.com> > Signed-off-by: David Vrabel > [bwh: Backported to 3.2: > - Adjust context > - gntdev_priv::lock is a spinlock rather than a mutex. As > gntdev_put_map() > may sleep, we need to unlock inside the loop.] > Signed-off-by: Ben Hutchings > --- > --- a/drivers/xen/gntdev.c > +++ b/drivers/xen/gntdev.c > @@ -493,11 +493,15 @@ static int gntdev_release(struct inode * > =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); > + spin_unlock(&priv->lock); > gntdev_put_map(map); > + spin_lock(&priv->lock); > } > + spin_unlock(&priv->lock); > =20 > if (use_ptemod) > mmu_notifier_unregister(&priv->mn, priv->mm); >=20