On Mon, 2015-10-12 at 11:11 +0100, David Vrabel wrote:
> On 08/10/15 23:14, Ben Hutchings wrote:
> > 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.
>
> Please just take commit 1401c00e59e instead.
I couldn't 'just' take that commit; it doesn't apply cleanly. However
I've backported it and changed 30b03d05e074 accordingly. The two
patches are attached for your review.
Ben.
--
Ben Hutchings
Teamwork is essential - it allows you to blame someone else.From: David Vrabel <david.vrabel@xxxxxxxxxx>
Date: Fri, 9 Jan 2015 18:06:12 +0000
Subject: xen/gntdev: convert priv->lock to a mutex
commit 1401c00e59ea021c575f74612fe2dbba36d6a4ee upstream.
Unmapping may require sleeping and we unmap while holding priv->lock, so
convert it to a mutex.
Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
Reviewed-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
[bwh: Backported to 3.2:
- Adjust context
- Drop changes to functions we don't have]
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
---
drivers/xen/gntdev.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -60,7 +60,7 @@ static int use_ptemod;
struct gntdev_priv {
struct list_head maps;
/* lock protects maps from concurrent changes */
- spinlock_t lock;
+ struct mutex lock;
struct mm_struct *mm;
struct mmu_notifier mn;
};
@@ -395,7 +395,7 @@ static void mn_invl_range_start(struct m
unsigned long mstart, mend;
int err;
- spin_lock(&priv->lock);
+ mutex_lock(&priv->lock);
list_for_each_entry(map, &priv->maps, next) {
if (!map->vma)
continue;
@@ -414,7 +414,7 @@ static void mn_invl_range_start(struct m
(mend - mstart) >> PAGE_SHIFT);
WARN_ON(err);
}
- spin_unlock(&priv->lock);
+ mutex_unlock(&priv->lock);
}
static void mn_invl_page(struct mmu_notifier *mn,
@@ -431,7 +431,7 @@ static void mn_release(struct mmu_notifi
struct grant_map *map;
int err;
- spin_lock(&priv->lock);
+ mutex_lock(&priv->lock);
list_for_each_entry(map, &priv->maps, next) {
if (!map->vma)
continue;
@@ -441,7 +441,7 @@ static void mn_release(struct mmu_notifi
err = unmap_grant_pages(map, /* offset */ 0, map->count);
WARN_ON(err);
}
- spin_unlock(&priv->lock);
+ mutex_unlock(&priv->lock);
}
struct mmu_notifier_ops gntdev_mmu_ops = {
@@ -462,7 +462,7 @@ static int gntdev_open(struct inode *ino
return -ENOMEM;
INIT_LIST_HEAD(&priv->maps);
- spin_lock_init(&priv->lock);
+ mutex_init(&priv->lock);
if (use_ptemod) {
priv->mm = get_task_mm(current);
@@ -535,10 +535,10 @@ static long gntdev_ioctl_map_grant_ref(s
return err;
}
- spin_lock(&priv->lock);
+ mutex_lock(&priv->lock);
gntdev_add_map(priv, map);
op.index = map->index << PAGE_SHIFT;
- spin_unlock(&priv->lock);
+ mutex_unlock(&priv->lock);
if (copy_to_user(u, &op, sizeof(op)) != 0)
return -EFAULT;
@@ -557,13 +557,13 @@ static long gntdev_ioctl_unmap_grant_ref
return -EFAULT;
pr_debug("priv %p, del %d+%d\n", priv, (int)op.index, (int)op.count);
- spin_lock(&priv->lock);
+ mutex_lock(&priv->lock);
map = gntdev_find_map_index(priv, op.index >> PAGE_SHIFT, op.count);
if (map) {
list_del(&map->next);
err = 0;
}
- spin_unlock(&priv->lock);
+ mutex_unlock(&priv->lock);
if (map)
gntdev_put_map(map);
return err;
@@ -608,7 +608,7 @@ static long gntdev_ioctl_notify(struct g
if (op.action & ~(UNMAP_NOTIFY_CLEAR_BYTE|UNMAP_NOTIFY_SEND_EVENT))
return -EINVAL;
- spin_lock(&priv->lock);
+ mutex_lock(&priv->lock);
list_for_each_entry(map, &priv->maps, next) {
uint64_t begin = map->index << PAGE_SHIFT;
@@ -631,7 +631,7 @@ static long gntdev_ioctl_notify(struct g
map->notify.event = op.event_channel_port;
rc = 0;
unlock_out:
- spin_unlock(&priv->lock);
+ mutex_unlock(&priv->lock);
return rc;
}
@@ -676,7 +676,7 @@ static int gntdev_mmap(struct file *flip
pr_debug("map %d+%d at %lx (pgoff %lx)\n",
index, count, vma->vm_start, vma->vm_pgoff);
- spin_lock(&priv->lock);
+ mutex_lock(&priv->lock);
map = gntdev_find_map_index(priv, index, count);
if (!map)
goto unlock_out;
@@ -711,7 +711,7 @@ static int gntdev_mmap(struct file *flip
map->flags |= GNTMAP_readonly;
}
- spin_unlock(&priv->lock);
+ mutex_unlock(&priv->lock);
if (use_ptemod) {
err = apply_to_page_range(vma->vm_mm, vma->vm_start,
@@ -739,11 +739,11 @@ static int gntdev_mmap(struct file *flip
return 0;
unlock_out:
- spin_unlock(&priv->lock);
+ mutex_unlock(&priv->lock);
return err;
out_unlock_put:
- spin_unlock(&priv->lock);
+ mutex_unlock(&priv->lock);
out_put_map:
if (use_ptemod)
map->vma = NULL;
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
<marmarek@xxxxxxxxxxxxxxxxxxxxxx>
Date: Fri, 26 Jun 2015 03:28:24 +0200
Subject: xen/gntdevt: Fix race condition in gntdev_release()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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]
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
---
drivers/xen/gntdev.c | 2 ++
1 file changed, 2 insertions(+)
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -493,11 +493,13 @@ static int gntdev_release(struct inode *
pr_debug("priv %p\n", priv);
+ mutex_lock(&priv->lock);
while (!list_empty(&priv->maps)) {
map = list_entry(priv->maps.next, struct grant_map, next);
list_del(&map->next);
gntdev_put_map(map);
}
+ mutex_unlock(&priv->lock);
if (use_ptemod)
mmu_notifier_unregister(&priv->mn, priv->mm);
Attachment:
signature.asc
Description: This is a digitally signed message part