[PATCH v2 0/6] mm, drm: fix interaction of .pfn_mkwrite() with fixup_user_fault()

This "v2" combines three series that I have previously posted separately to gather
reviews and tests:

- kvm: apply VM_READ/VM_WRITE checks to all VMA types
  https://lore.kernel.org/kvm/[email protected]/

- mm: pull writability check to follow_pfnmap_start()
  https://lore.kernel.org/kvm/[email protected]/T/#u

- mm, drm: ensure .fault() does not have to be followed by .pfn_mkwrite() for write faults
  https://lore.kernel.org/kvm/[email protected]/

All three, together, make it possible to write drivers that use .fault()
and .pfn_mkwrite() callbacks for VM_IO|VM_PFNMAP regions, and that also
interact correctly with users of fixup_user_fault().

The problem is that if you define .pfn_mkwrite(), vma_set_page_prot()
clears the writable PTE bit in vma->vm_page_prot, at which point
the .fault() callback has no way to create a writable PTE.  Users of
fixup_user_fault() will then see a read-only PTE and have no clue that
the page needs a *second* fault to reach its final status.

fixup_user_fault() itself does not have a good way to notice this, because
vma->vm_page_prot is an opaque pgprot_t, so the fix needs to be somewhere
else.  Other preexisting functions, namely vmf_insert_page_mkwrite()
as well as vmf_insert_pfn_pmd(), suggest that this has to be the driver.
In fact, of the five vm_ops that use .pfn_mkwrite() together with .fault(),
three are in file systems and are not buggy: all of them ultimately end
up in dax_fault_iter(), which uses vmf_insert_page_mkwrite() to correctly
insert the PTE.

The two problematic implementations instead are both in drm code.  One, in
drm_gem_shmem_helper, was reported as a KVM regression; the other, in
vmwgfx, was found by inspection of .pfn_mkwrite() implementors.
Both of these use pfn-mapped regions, but there is nothing like a
vmf_insert_pfn_mkwrite() function that they could use; the first part of
this series thus adjusts mm.h to provide two new functions for this
usecase---vmf_insert_pfn_prot_mkwrite() and vmf_insert_pfn_mkwrite()---and
then teaches drm's two users of .pfn_mkwrite() to call them.

This however leaves another case buggy where fixup_user_fault() is preceded
by follow_pfnmap_start().  Most callers of follow_pfnmap_start(), seeing
it return 0 for a PFN that is mapped read-only, would not attempt to
call fixup_user_fault() on it, and thus the PTE would not be upgraded
to writable.

This is arguably a bug in... almost all the callers of follow_pfnmap_start(),
but fixing it is much better achieved with a small improvement to the API;
if follow_pfnmap_start() is told by the caller that it needs the memory
for a write, most callers are simplified because they were doing such a check
anyway and now just see -EFAULT.  They then proceed to call fixup_user_fault()
and everyone is happy.  This is done in patch 5.

To sum up:

- patches 1-3 introduce the new MM API, and use it in the DRM .fault()
  callbacks to install writable PTEs in response to write faults

- patch 4 is a preparatory fix in KVM, eliminating inconsistencies in the
  handling of !VM_READ and !VM_WRITE VMAs; these would return different
  error codes for a !VM_WRITE VMA depending on whether the PTE happens
  to be mapped (but with wrong permissions).  This needs to be here
  because the next patch would introduce even more inconsistencies.
  
- patch 5 moves the check for writable PTEs from follow_pfnmap_start()'s
  callers to the function itself

- finally, patch 6 is a KVM addendum that will have to wait until
  the next merge window; it removes yet another inconsistency in
  KVM's handling of !VM_WRITE but is technically userspace-visible,
  and therefore it shouldn't be included in stable kernel releases
  unlike the rest.


Thanks,

Paolo


Paolo Bonzini (6):
  mm: export vmf_insert_pfn_prot_mkwrite(), change variants to inline
  drm/shmem_helper: use vmf_insert_pfn_mkwrite()
  drm/ttm, drm/vmwgfx: directly create writable PTEs when mkwrite is in
    use
  kvm: apply VM_READ/VM_WRITE checks to all VMA types
  mm: pull writability check to follow_pfnmap_start()
  kvm: return -EFAULT for writes to !VM_WRITE IO mappings

 arch/s390/pci/pci_mmio.c                   |   2 +
 drivers/gpu/drm/drm_gem_shmem_helper.c     |  38 +++---
 drivers/gpu/drm/ttm/ttm_bo_vm.c            |   7 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c |  42 +++---
 drivers/vfio/vfio_iommu_type1.c            |  17 ++-
 drivers/virt/acrn/mm.c                     |  10 +-
 include/linux/mm.h                         |  84 +++++++++++-
 mm/huge_memory.c                           |   2 +-
 mm/memory.c                                | 146 +++++++++++----------
 virt/kvm/kvm_main.c                        |  44 +++----
 10 files changed, 232 insertions(+), 160 deletions(-)

-- 
2.55.0