mbox series

[v7,00/17] follow_pfn and other iomap races

Message ID 20201127164131.2244124-1-daniel.vetter@ffwll.ch
Headers show
Series follow_pfn and other iomap races | expand

Message

Daniel Vetter Nov. 27, 2020, 4:41 p.m. UTC
Hi all

Another update of my patch series to clamp down a bunch of races and gaps
around follow_pfn and other access to iomem mmaps. Previous version:

v1: https://lore.kernel.org/dri-devel/20201007164426.1812530-1-daniel.vetter@ffwll.ch/
v2: https://lore.kernel.org/dri-devel/20201009075934.3509076-1-daniel.vetter@ffwll.ch
v3: https://lore.kernel.org/dri-devel/20201021085655.1192025-1-daniel.vetter@ffwll.ch/
v4: https://lore.kernel.org/dri-devel/20201026105818.2585306-1-daniel.vetter@ffwll.ch/
v5: https://lore.kernel.org/dri-devel/20201030100815.2269-1-daniel.vetter@ffwll.ch/
v6: https://lore.kernel.org/dri-devel/20201119144146.1045202-1-daniel.vetter@ffwll.ch/

And the discussion that sparked this journey:

https://lore.kernel.org/dri-devel/20201007164426.1812530-1-daniel.vetter@ffwll.ch/

I think the first 12 patches are ready for landing. The parts starting
with "mm: Add unsafe_follow_pfn" probably need more baking time.

Andrew, can you please pick these up, or do you prefer I do a topic branch
and send them to Linus directly in the next merge window?

Changes in v7:
- more acks/reviews
- reordered with the ready pieces at the front
- simplified the new follow_pfn function as Jason suggested

Changes in v6:
- Tested v4l userptr as Tomasz suggested. No boom observed
- Added RFC for locking down follow_pfn, per discussion with Christoph and
  Jason.
- Explain why pup_fast is safe in relevant patches, there was a bit a
  confusion when discussing v5.
- Fix up the resource patch, with CONFIG_IO_STRICT_DEVMEM it crashed on
  boot due to an unintended change (reported by John)

Changes in v5:
- Tomasz found some issues in the media patches
- Polish suggested by Christoph for the unsafe_follow_pfn patch

Changes in v4:
- Drop the s390 patch, that was very stand-alone and now queued up to land
  through s390 trees.
- Comment polish per Dan's review.

Changes in v3:
- Bunch of polish all over, no functional changes aside from one barrier
  in the resource code, for consistency.
- A few more r-b tags.

Changes in v2:
- tons of small polish&fixes all over, thanks to all the reviewers who
  spotted issues
- I managed to test at least the generic_access_phys and pci mmap revoke
  stuff with a few gdb sessions using our i915 debug tools (hence now also
  the drm/i915 patch to properly request all the pci bar regions)
- reworked approach for the pci mmap revoke: Infrastructure moved into
  kernel/resource.c, address_space mapping is now set up at open time for
  everyone (which required some sysfs changes). Does indeed look a lot
  cleaner and a lot less invasive than I feared at first.

Coments and review on the remaining bits very much welcome, especially
from the kvm and vfio side.

Cheers, Daniel

Daniel Vetter (17):
  drm/exynos: Stop using frame_vector helpers
  drm/exynos: Use FOLL_LONGTERM for g2d cmdlists
  misc/habana: Stop using frame_vector helpers
  misc/habana: Use FOLL_LONGTERM for userptr
  mm/frame-vector: Use FOLL_LONGTERM
  media: videobuf2: Move frame_vector into media subsystem
  mm: Close race in generic_access_phys
  PCI: Obey iomem restrictions for procfs mmap
  /dev/mem: Only set filp->f_mapping
  resource: Move devmem revoke code to resource framework
  sysfs: Support zapping of binary attr mmaps
  PCI: Revoke mappings like devmem
  mm: Add unsafe_follow_pfn
  media/videobuf1|2: Mark follow_pfn usage as unsafe
  vfio/type1: Mark follow_pfn as unsafe
  kvm: pass kvm argument to follow_pfn callsites
  mm: add mmu_notifier argument to follow_pfn

 arch/powerpc/kvm/book3s_64_mmu_hv.c           |   2 +-
 arch/powerpc/kvm/book3s_64_mmu_radix.c        |   2 +-
 arch/powerpc/kvm/e500_mmu_host.c              |   2 +-
 arch/x86/kvm/mmu/mmu.c                        |   8 +-
 drivers/char/mem.c                            |  86 +-------------
 drivers/gpu/drm/exynos/Kconfig                |   1 -
 drivers/gpu/drm/exynos/exynos_drm_g2d.c       |  48 ++++----
 drivers/media/common/videobuf2/Kconfig        |   1 -
 drivers/media/common/videobuf2/Makefile       |   1 +
 .../media/common/videobuf2}/frame_vector.c    |  57 ++++-----
 .../media/common/videobuf2/videobuf2-memops.c |   3 +-
 drivers/media/platform/omap/Kconfig           |   1 -
 drivers/media/v4l2-core/videobuf-dma-contig.c |   2 +-
 drivers/misc/habanalabs/Kconfig               |   1 -
 drivers/misc/habanalabs/common/habanalabs.h   |   6 +-
 drivers/misc/habanalabs/common/memory.c       |  52 +++-----
 drivers/pci/pci-sysfs.c                       |   4 +
 drivers/pci/proc.c                            |   6 +
 drivers/vfio/vfio_iommu_type1.c               |   4 +-
 fs/sysfs/file.c                               |  11 ++
 include/linux/ioport.h                        |   6 +-
 include/linux/kvm_host.h                      |   9 +-
 include/linux/mm.h                            |  50 +-------
 include/linux/sysfs.h                         |   2 +
 include/media/frame_vector.h                  |  47 ++++++++
 include/media/videobuf2-core.h                |   1 +
 kernel/resource.c                             |  98 ++++++++++++++-
 mm/Kconfig                                    |   3 -
 mm/Makefile                                   |   1 -
 mm/memory.c                                   | 112 +++++++++++++++---
 mm/nommu.c                                    |  16 ++-
 security/Kconfig                              |  13 ++
 virt/kvm/kvm_main.c                           |  56 +++++----
 33 files changed, 413 insertions(+), 299 deletions(-)
 rename {mm => drivers/media/common/videobuf2}/frame_vector.c (84%)
 create mode 100644 include/media/frame_vector.h

Comments

Daniel Vetter Jan. 12, 2021, 1:24 p.m. UTC | #1
On Fri, Nov 27, 2020 at 05:41:14PM +0100, Daniel Vetter wrote:
> Hi all
> 
> Another update of my patch series to clamp down a bunch of races and gaps
> around follow_pfn and other access to iomem mmaps. Previous version:
> 
> v1: https://lore.kernel.org/dri-devel/20201007164426.1812530-1-daniel.vetter@ffwll.ch/
> v2: https://lore.kernel.org/dri-devel/20201009075934.3509076-1-daniel.vetter@ffwll.ch
> v3: https://lore.kernel.org/dri-devel/20201021085655.1192025-1-daniel.vetter@ffwll.ch/
> v4: https://lore.kernel.org/dri-devel/20201026105818.2585306-1-daniel.vetter@ffwll.ch/
> v5: https://lore.kernel.org/dri-devel/20201030100815.2269-1-daniel.vetter@ffwll.ch/
> v6: https://lore.kernel.org/dri-devel/20201119144146.1045202-1-daniel.vetter@ffwll.ch/
> 
> And the discussion that sparked this journey:
> 
> https://lore.kernel.org/dri-devel/20201007164426.1812530-1-daniel.vetter@ffwll.ch/
> 
> I think the first 12 patches are ready for landing. The parts starting
> with "mm: Add unsafe_follow_pfn" probably need more baking time.
> 
> Andrew, can you please pick these up, or do you prefer I do a topic branch
> and send them to Linus directly in the next merge window?
> 
> Changes in v7:
> - more acks/reviews
> - reordered with the ready pieces at the front
> - simplified the new follow_pfn function as Jason suggested
> 
> Changes in v6:
> - Tested v4l userptr as Tomasz suggested. No boom observed
> - Added RFC for locking down follow_pfn, per discussion with Christoph and
>   Jason.
> - Explain why pup_fast is safe in relevant patches, there was a bit a
>   confusion when discussing v5.
> - Fix up the resource patch, with CONFIG_IO_STRICT_DEVMEM it crashed on
>   boot due to an unintended change (reported by John)
> 
> Changes in v5:
> - Tomasz found some issues in the media patches
> - Polish suggested by Christoph for the unsafe_follow_pfn patch
> 
> Changes in v4:
> - Drop the s390 patch, that was very stand-alone and now queued up to land
>   through s390 trees.
> - Comment polish per Dan's review.
> 
> Changes in v3:
> - Bunch of polish all over, no functional changes aside from one barrier
>   in the resource code, for consistency.
> - A few more r-b tags.
> 
> Changes in v2:
> - tons of small polish&fixes all over, thanks to all the reviewers who
>   spotted issues
> - I managed to test at least the generic_access_phys and pci mmap revoke
>   stuff with a few gdb sessions using our i915 debug tools (hence now also
>   the drm/i915 patch to properly request all the pci bar regions)
> - reworked approach for the pci mmap revoke: Infrastructure moved into
>   kernel/resource.c, address_space mapping is now set up at open time for
>   everyone (which required some sysfs changes). Does indeed look a lot
>   cleaner and a lot less invasive than I feared at first.
> 
> Coments and review on the remaining bits very much welcome, especially
> from the kvm and vfio side.
> 
> Cheers, Daniel
> 
> Daniel Vetter (17):
>   drm/exynos: Stop using frame_vector helpers
>   drm/exynos: Use FOLL_LONGTERM for g2d cmdlists
>   misc/habana: Stop using frame_vector helpers
>   misc/habana: Use FOLL_LONGTERM for userptr
>   mm/frame-vector: Use FOLL_LONGTERM
>   media: videobuf2: Move frame_vector into media subsystem
>   mm: Close race in generic_access_phys
>   PCI: Obey iomem restrictions for procfs mmap
>   /dev/mem: Only set filp->f_mapping
>   resource: Move devmem revoke code to resource framework
>   sysfs: Support zapping of binary attr mmaps
>   PCI: Revoke mappings like devmem

As Jason suggested, I've pulled the first 1 patches into a topic branch.

Stephen, can you please add the below to linux-next for the 5.12 merge
window?

git://anongit.freedesktop.org/drm/drm topic/iomem-mmap-vs-gup

Once this part has landed I'll see what to do with the below part.

Thanks, Daniel

>   mm: Add unsafe_follow_pfn
>   media/videobuf1|2: Mark follow_pfn usage as unsafe
>   vfio/type1: Mark follow_pfn as unsafe
>   kvm: pass kvm argument to follow_pfn callsites
>   mm: add mmu_notifier argument to follow_pfn
> 
>  arch/powerpc/kvm/book3s_64_mmu_hv.c           |   2 +-
>  arch/powerpc/kvm/book3s_64_mmu_radix.c        |   2 +-
>  arch/powerpc/kvm/e500_mmu_host.c              |   2 +-
>  arch/x86/kvm/mmu/mmu.c                        |   8 +-
>  drivers/char/mem.c                            |  86 +-------------
>  drivers/gpu/drm/exynos/Kconfig                |   1 -
>  drivers/gpu/drm/exynos/exynos_drm_g2d.c       |  48 ++++----
>  drivers/media/common/videobuf2/Kconfig        |   1 -
>  drivers/media/common/videobuf2/Makefile       |   1 +
>  .../media/common/videobuf2}/frame_vector.c    |  57 ++++-----
>  .../media/common/videobuf2/videobuf2-memops.c |   3 +-
>  drivers/media/platform/omap/Kconfig           |   1 -
>  drivers/media/v4l2-core/videobuf-dma-contig.c |   2 +-
>  drivers/misc/habanalabs/Kconfig               |   1 -
>  drivers/misc/habanalabs/common/habanalabs.h   |   6 +-
>  drivers/misc/habanalabs/common/memory.c       |  52 +++-----
>  drivers/pci/pci-sysfs.c                       |   4 +
>  drivers/pci/proc.c                            |   6 +
>  drivers/vfio/vfio_iommu_type1.c               |   4 +-
>  fs/sysfs/file.c                               |  11 ++
>  include/linux/ioport.h                        |   6 +-
>  include/linux/kvm_host.h                      |   9 +-
>  include/linux/mm.h                            |  50 +-------
>  include/linux/sysfs.h                         |   2 +
>  include/media/frame_vector.h                  |  47 ++++++++
>  include/media/videobuf2-core.h                |   1 +
>  kernel/resource.c                             |  98 ++++++++++++++-
>  mm/Kconfig                                    |   3 -
>  mm/Makefile                                   |   1 -
>  mm/memory.c                                   | 112 +++++++++++++++---
>  mm/nommu.c                                    |  16 ++-
>  security/Kconfig                              |  13 ++
>  virt/kvm/kvm_main.c                           |  56 +++++----
>  33 files changed, 413 insertions(+), 299 deletions(-)
>  rename {mm => drivers/media/common/videobuf2}/frame_vector.c (84%)
>  create mode 100644 include/media/frame_vector.h
> 
> -- 
> 2.29.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Stephen Rothwell Jan. 12, 2021, 8:57 p.m. UTC | #2
Hi Daniel,

On Tue, 12 Jan 2021 14:24:27 +0100 Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> As Jason suggested, I've pulled the first 1 patches into a topic branch.
> 
> Stephen, can you please add the below to linux-next for the 5.12 merge
> window?
> 
> git://anongit.freedesktop.org/drm/drm topic/iomem-mmap-vs-gup

Added from today.

Thanks for adding your subsystem tree as a participant of linux-next.  As
you may know, this is not a judgement of your code.  The purpose of
linux-next is for integration testing and to lower the impact of
conflicts between subsystems in the next merge window. 

You will need to ensure that the patches/commits in your tree/series have
been:
     * submitted under GPL v2 (or later) and include the Contributor's
        Signed-off-by,
     * posted to the relevant mailing list,
     * reviewed by you (or another maintainer of your subsystem tree),
     * successfully unit tested, and 
     * destined for the current or next Linux merge window.

Basically, this should be just what you would send to Linus (or ask him
to fetch).  It is allowed to be rebased if you deem it necessary.