lkml.org 
[lkml]   [2022]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 2/2] xen/gntdev: Accommodate VMA splitting
From
On 12.09.22 06:00, M. Vefa Bicakci wrote:
> Prior to this commit, the gntdev driver code did not handle the
> following scenario correctly:
>
> * 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
> dom0:
>
> (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 (Xen reports that a granted page is being
> unmapped and triggers a general protection fault in dom0
> 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@vger.kernel.org
> Signed-off-by: M. Vefa Bicakci <m.v.b@runbox.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2022-09-19 12:47    [W:0.234 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site