On 03.10.22 00:20, M. Vefa Bicakci wrote: > 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: > > 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@vger.kernel.org > Signed-off-by: M. Vefa Bicakci Reviewed-by: Juergen Gross Juergen