Re: [Xen-devel] [linux-3.14 bisection] complete test-amd64-i386-xl-qcow2

[Ben Hutchings <ben@xxxxxxxxxxxxxxx>]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel