[v6,0/6] remove the vmas parameter from GUP APIs

Message

Lorenzo Stoakes May 17, 2023, 7:25 p.m. UTC

(pin_/get)_user_pages[_remote]() each provide an optional output parameter
for an array of VMA objects associated with each page in the input range.

These provide the means for VMAs to be returned, as long as mm->mmap_lock
is never released during the GUP operation (i.e. the internal flag
FOLL_UNLOCKABLE is not specified).

In addition, these VMAs can only be accessed with the mmap_lock held and
become invalidated the moment it is released.

The vast majority of invocations do not use this functionality and of those
that do, all but one case retrieve a single VMA to perform checks upon.

It is not egregious in the single VMA cases to simply replace the operation
with a vma_lookup(). In these cases we duplicate the (fast) lookup on a
slow path already under the mmap_lock, abstracted to a new
get_user_page_vma_remote() inline helper function which also performs error
checking and reference count maintenance.

The special case is io_uring, where io_pin_pages() specifically needs to
assert that the VMAs underlying the range do not result in broken long-term
GUP file-backed mappings.

As GUP now internally asserts that FOLL_LONGTERM mappings are not
file-backed in a broken fashion (i.e. requiring dirty tracking) - as
implemented in "mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to
file-backed mappings" - this logic is no longer required and so we can
simply remove it altogether from io_uring.

Eliminating the vmas parameter eliminates an entire class of danging
pointer errors that might have occured should the lock have been
incorrectly released.

In addition, the API is simplified and now clearly expresses what it is
intended for - applying the specified GUP flags and (if pinning) returning
pinned pages.

This change additionally opens the door to further potential improvements
in GUP and the possible marrying of disparate code paths.

I have run this series against gup_test with no issues.

This patch series is rebased on mm-unstable as of 12th May.

Thanks to Matthew Wilcox for suggesting this refactoring!

v6:
- Fixed up unused variable warning under !CONFIG_HAVE_IOREMAP_PROT
  reported by Anders Roxell.
- Fixed up use of uninitialised vma parameter on error/no page found
  reported by Dan Carpenter (caused by reuse of code with helper that sets
  vma under different circumstances).
- Fixed up the missed kernel/trace/trace_events_user.c caller.

v5:
- Remove the io_uring open-coded VMA file-backed check, as this is now
  explicitly disallowed by GUP.
- Updated the subsequent patch to eliminate the vmas parameter accordingly.
https://lore.kernel/org/linux-mm/[e-mail]/

v4:
- Drop FOLL_SAME_FILE as the complexity costs exceed the benefit of having it
  for a single case.
- Update io_pin_pages() to perform VMA lookup directly.
- Add get_user_page_vma_remote() to perform the single page/VMA lookup with
  error checks performed correctly.
https://lore.kernel.org/linux-mm/[e-mail]/

v3:
- Always explicitly handle !vma cases, feeding back an error to the user if
  appropriate, indicating the operation did not completely succeed if not
  and always with a warning since these conditions should be impossible.
https://lore.kernel.org/linux-mm/[e-mail]/

v2:
- Only lookup the VMA if the pin succeeded (other than __access_remote_vm()
  which has different semantics)
- Be pedantically careful about ensuring that under no circumstances can we
  fail to unpin a page
https://lore.kernel.org/linux-mm/[e-mail]/

v1:
https://lore.kernel.org/linux-mm/[e-mail]/

Lorenzo Stoakes (6):
  mm/gup: remove unused vmas parameter from get_user_pages()
  mm/gup: remove unused vmas parameter from pin_user_pages_remote()
  mm/gup: remove vmas parameter from get_user_pages_remote()
  io_uring: rsrc: delegate VMA file-backed check to GUP
  mm/gup: remove vmas parameter from pin_user_pages()
  mm/gup: remove vmas array from internal GUP functions

 arch/arm64/kernel/mte.c                    |  17 ++--
 arch/powerpc/mm/book3s64/iommu_api.c       |   2 +-
 arch/s390/kvm/interrupt.c                  |   2 +-
 arch/x86/kernel/cpu/sgx/ioctl.c            |   2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c        |   2 +-
 drivers/infiniband/hw/qib/qib_user_pages.c |   2 +-
 drivers/infiniband/hw/usnic/usnic_uiom.c   |   2 +-
 drivers/infiniband/sw/siw/siw_mem.c        |   2 +-
 drivers/iommu/iommufd/pages.c              |   4 +-
 drivers/media/v4l2-core/videobuf-dma-sg.c  |   2 +-
 drivers/misc/sgi-gru/grufault.c            |   2 +-
 drivers/vdpa/vdpa_user/vduse_dev.c         |   2 +-
 drivers/vfio/vfio_iommu_type1.c            |   2 +-
 drivers/vhost/vdpa.c                       |   2 +-
 fs/exec.c                                  |   2 +-
 include/linux/hugetlb.h                    |  10 +-
 include/linux/mm.h                         |  42 +++++++--
 io_uring/rsrc.c                            |  34 ++-----
 kernel/events/uprobes.c                    |  13 +--
 kernel/trace/trace_events_user.c           |   2 +-
 mm/gup.c                                   | 105 +++++++--------------
 mm/gup_test.c                              |  14 ++-
 mm/hugetlb.c                               |  24 ++---
 mm/memory.c                                |  20 ++--
 mm/process_vm_access.c                     |   2 +-
 mm/rmap.c                                  |   2 +-
 net/xdp/xdp_umem.c                         |   2 +-
 security/tomoyo/domain.c                   |   2 +-
 virt/kvm/async_pf.c                        |   3 +-
 virt/kvm/kvm_main.c                        |   2 +-
 30 files changed, 142 insertions(+), 182 deletions(-)

--
2.40.1