On Wed, 2015-09-02 at 10:18 +0100, Ian Campbell wrote: > [resending to correct stable address, sorry folks] > > TL;DR: Any backport of 30b03d05e074 to earlier than commit 1401c00e59e > ("xen/gntdev: convert priv->lock to a mutex", which was added in v4.0) > needs $something doing to it, either s/mutex/spinlock/ or (more likely) > backporting of 1401c00e59e too. > > Looking at LTS: > > 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 > > So AFAICT 3.14.y and 3.10.y need fixes, probably following 3.18 and > backporting 1401c00e59e. > > 3.16/12/4/2 might need to be careful if they subsequently pick up 30b03d05. [...] I came up with the patch below for 3.2. Let me know if it's not correct. Ben. --- From: Marek Marczykowski-Górecki Date: Fri, 26 Jun 2015 03:28:24 +0200 Subject: xen/gntdevt: Fix race condition in gntdev_release() commit 30b03d05e07467b8c6ec683ea96b5bffcbcd3931 upstream. 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. Signed-off-by: Marek Marczykowski-Górecki 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 * pr_debug("priv %p\n", priv); + spin_lock(&priv->lock); while (!list_empty(&priv->maps)) { map = 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); if (use_ptemod) mmu_notifier_unregister(&priv->mn, priv->mm); -- Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse.