Patch "xen/gntdev: Accommodate VMA splitting" has been added to the 5.10-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    xen/gntdev: Accommodate VMA splitting

to the 5.10-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     xen-gntdev-accommodate-vma-splitting.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit e4fb93409b5f43ec8c06d049b1901a2e4b1f2870
Author: M. Vefa Bicakci <m.v.b@xxxxxxxxxx>
Date:   Sun Oct 2 18:20:06 2022 -0400

    xen/gntdev: Accommodate VMA splitting
    
    [ Upstream commit 5c13a4a0291b30191eff9ead8d010e1ca43a4d0c ]
    
    Prior to this commit, the gntdev driver code did not handle the
    following scenario correctly with paravirtualized (PV) Xen domains:
    
    * User process sets up a gntdev mapping composed of two grant mappings
      (i.e., two pages shared by another Xen domain).
    * User process munmap()s one of the pages.
    * User process munmap()s the remaining page.
    * User process exits.
    
    In the scenario above, the user process would cause the kernel to log
    the following messages in dmesg for the first munmap(), and the second
    munmap() call would result in similar log messages:
    
      BUG: Bad page map in process doublemap.test  pte:... pmd:...
      page:0000000057c97bff refcount:1 mapcount:-1 \
        mapping:0000000000000000 index:0x0 pfn:...
      ...
      page dumped because: bad pte
      ...
      file:gntdev fault:0x0 mmap:gntdev_mmap [xen_gntdev] readpage:0x0
      ...
      Call Trace:
       <TASK>
       dump_stack_lvl+0x46/0x5e
       print_bad_pte.cold+0x66/0xb6
       unmap_page_range+0x7e5/0xdc0
       unmap_vmas+0x78/0xf0
       unmap_region+0xa8/0x110
       __do_munmap+0x1ea/0x4e0
       __vm_munmap+0x75/0x120
       __x64_sys_munmap+0x28/0x40
       do_syscall_64+0x38/0x90
       entry_SYSCALL_64_after_hwframe+0x61/0xcb
       ...
    
    For each munmap() call, the Xen hypervisor (if built with CONFIG_DEBUG)
    would print out the following and trigger a general protection fault in
    the affected Xen PV domain:
    
      (XEN) d0v... Attempt to implicitly unmap d0's grant PTE ...
      (XEN) d0v... Attempt to implicitly unmap d0's grant PTE ...
    
    As of this writing, gntdev_grant_map structure's vma field (referred to
    as map->vma below) is mainly used for checking the start and end
    addresses of mappings. However, with split VMAs, these may change, and
    there could be more than one VMA associated with a gntdev mapping.
    Hence, remove the use of map->vma and rely on map->pages_vm_start for
    the original start address and on (map->count << PAGE_SHIFT) for the
    original mapping size. Let the invalidate() and find_special_page()
    hooks use these.
    
    Also, given that there can be multiple VMAs associated with a gntdev
    mapping, move the "mmu_interval_notifier_remove(&map->notifier)" call to
    the end of gntdev_put_map, so that the MMU notifier is only removed
    after the closing of the last remaining VMA.
    
    Finally, use an atomic to prevent inadvertent gntdev mapping re-use,
    instead of using the map->live_grants atomic counter and/or the map->vma
    pointer (the latter of which is now removed). This prevents the
    userspace from mmap()'ing (with MAP_FIXED) a gntdev mapping over the
    same address range as a previously set up gntdev mapping. This scenario
    can be summarized with the following call-trace, which was valid prior
    to this commit:
    
      mmap
        gntdev_mmap
      mmap (repeat mmap with MAP_FIXED over the same address range)
        gntdev_invalidate
          unmap_grant_pages (sets 'being_removed' entries to true)
            gnttab_unmap_refs_async
        unmap_single_vma
        gntdev_mmap (maps the shared pages again)
      munmap
        gntdev_invalidate
          unmap_grant_pages
            (no-op because 'being_removed' entries are true)
        unmap_single_vma (For PV domains, Xen reports that a granted page
          is being unmapped and triggers a general protection fault in the
          affected domain, if Xen was built with CONFIG_DEBUG)
    
    The fix for this last scenario could be worth its own commit, but we
    opted for a single commit, because removing the gntdev_grant_map
    structure's vma field requires guarding the entry to gntdev_mmap(), and
    the live_grants atomic counter is not sufficient on its own to prevent
    the mmap() over a pre-existing mapping.
    
    Link: https://github.com/QubesOS/qubes-issues/issues/7631
    Fixes: ab31523c2fca ("xen/gntdev: allow usermode to map granted pages")
    Cc: stable@xxxxxxxxxxxxxxx
    Signed-off-by: M. Vefa Bicakci <m.v.b@xxxxxxxxxx>
    Reviewed-by: Juergen Gross <jgross@xxxxxxxx>
    Link: https://lore.kernel.org/r/20221002222006.2077-3-m.v.b@xxxxxxxxxx
    Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/drivers/xen/gntdev-common.h b/drivers/xen/gntdev-common.h
index 40ef379c28ab..9c286b2a1900 100644
--- a/drivers/xen/gntdev-common.h
+++ b/drivers/xen/gntdev-common.h
@@ -44,9 +44,10 @@ struct gntdev_unmap_notify {
 };
 
 struct gntdev_grant_map {
+	atomic_t in_use;
 	struct mmu_interval_notifier notifier;
+	bool notifier_init;
 	struct list_head next;
-	struct vm_area_struct *vma;
 	int index;
 	int count;
 	int flags;
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 5dd9d1ac755e..ff195b571763 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -276,6 +276,9 @@ void gntdev_put_map(struct gntdev_priv *priv, struct gntdev_grant_map *map)
 		 */
 	}
 
+	if (use_ptemod && map->notifier_init)
+		mmu_interval_notifier_remove(&map->notifier);
+
 	if (map->notify.flags & UNMAP_NOTIFY_SEND_EVENT) {
 		notify_remote_via_evtchn(map->notify.event);
 		evtchn_put(map->notify.event);
@@ -288,7 +291,7 @@ void gntdev_put_map(struct gntdev_priv *priv, struct gntdev_grant_map *map)
 static int find_grant_ptes(pte_t *pte, unsigned long addr, void *data)
 {
 	struct gntdev_grant_map *map = data;
-	unsigned int pgnr = (addr - map->vma->vm_start) >> PAGE_SHIFT;
+	unsigned int pgnr = (addr - map->pages_vm_start) >> PAGE_SHIFT;
 	int flags = map->flags | GNTMAP_application_map | GNTMAP_contains_pte |
 		    (1 << _GNTMAP_guest_avail0);
 	u64 pte_maddr;
@@ -478,11 +481,7 @@ static void gntdev_vma_close(struct vm_area_struct *vma)
 	struct gntdev_priv *priv = file->private_data;
 
 	pr_debug("gntdev_vma_close %p\n", vma);
-	if (use_ptemod) {
-		WARN_ON(map->vma != vma);
-		mmu_interval_notifier_remove(&map->notifier);
-		map->vma = NULL;
-	}
+
 	vma->vm_private_data = NULL;
 	gntdev_put_map(priv, map);
 }
@@ -510,29 +509,30 @@ static bool gntdev_invalidate(struct mmu_interval_notifier *mn,
 	struct gntdev_grant_map *map =
 		container_of(mn, struct gntdev_grant_map, notifier);
 	unsigned long mstart, mend;
+	unsigned long map_start, map_end;
 
 	if (!mmu_notifier_range_blockable(range))
 		return false;
 
+	map_start = map->pages_vm_start;
+	map_end = map->pages_vm_start + (map->count << PAGE_SHIFT);
+
 	/*
 	 * If the VMA is split or otherwise changed the notifier is not
 	 * updated, but we don't want to process VA's outside the modified
 	 * VMA. FIXME: It would be much more understandable to just prevent
 	 * modifying the VMA in the first place.
 	 */
-	if (map->vma->vm_start >= range->end ||
-	    map->vma->vm_end <= range->start)
+	if (map_start >= range->end || map_end <= range->start)
 		return true;
 
-	mstart = max(range->start, map->vma->vm_start);
-	mend = min(range->end, map->vma->vm_end);
+	mstart = max(range->start, map_start);
+	mend = min(range->end, map_end);
 	pr_debug("map %d+%d (%lx %lx), range %lx %lx, mrange %lx %lx\n",
-			map->index, map->count,
-			map->vma->vm_start, map->vma->vm_end,
-			range->start, range->end, mstart, mend);
-	unmap_grant_pages(map,
-				(mstart - map->vma->vm_start) >> PAGE_SHIFT,
-				(mend - mstart) >> PAGE_SHIFT);
+		 map->index, map->count, map_start, map_end,
+		 range->start, range->end, mstart, mend);
+	unmap_grant_pages(map, (mstart - map_start) >> PAGE_SHIFT,
+			  (mend - mstart) >> PAGE_SHIFT);
 
 	return true;
 }
@@ -1012,18 +1012,15 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma)
 		return -EINVAL;
 
 	pr_debug("map %d+%d at %lx (pgoff %lx)\n",
-			index, count, vma->vm_start, vma->vm_pgoff);
+		 index, count, vma->vm_start, vma->vm_pgoff);
 
 	mutex_lock(&priv->lock);
 	map = gntdev_find_map_index(priv, index, count);
 	if (!map)
 		goto unlock_out;
-	if (use_ptemod && map->vma)
-		goto unlock_out;
-	if (atomic_read(&map->live_grants)) {
-		err = -EAGAIN;
+	if (!atomic_add_unless(&map->in_use, 1, 1))
 		goto unlock_out;
-	}
+
 	refcount_inc(&map->users);
 
 	vma->vm_ops = &gntdev_vmops;
@@ -1044,15 +1041,16 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma)
 			map->flags |= GNTMAP_readonly;
 	}
 
+	map->pages_vm_start = vma->vm_start;
+
 	if (use_ptemod) {
-		map->vma = vma;
 		err = mmu_interval_notifier_insert_locked(
 			&map->notifier, vma->vm_mm, vma->vm_start,
 			vma->vm_end - vma->vm_start, &gntdev_mmu_ops);
-		if (err) {
-			map->vma = NULL;
+		if (err)
 			goto out_unlock_put;
-		}
+
+		map->notifier_init = true;
 	}
 	mutex_unlock(&priv->lock);
 
@@ -1069,7 +1067,6 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma)
 		 */
 		mmu_interval_read_begin(&map->notifier);
 
-		map->pages_vm_start = vma->vm_start;
 		err = apply_to_page_range(vma->vm_mm, vma->vm_start,
 					  vma->vm_end - vma->vm_start,
 					  find_grant_ptes, map);
@@ -1098,13 +1095,8 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma)
 out_unlock_put:
 	mutex_unlock(&priv->lock);
 out_put_map:
-	if (use_ptemod) {
+	if (use_ptemod)
 		unmap_grant_pages(map, 0, map->count);
-		if (map->vma) {
-			mmu_interval_notifier_remove(&map->notifier);
-			map->vma = NULL;
-		}
-	}
 	gntdev_put_map(priv, map);
 	return err;
 }



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux