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 <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
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 <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
[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 <ben@xxxxxxxxxxxxxxx>
---
--- 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.
Attachment:
signature.asc
Description: This is a digitally signed message part