On Wed, Oct 7, 2020 at 7:39 PM Jason Gunthorpe <[email protected]> wrote: > > On Wed, Oct 07, 2020 at 06:44:26PM +0200, Daniel Vetter wrote: > > The code seems to stuff these pfns into iommu pts (or something like > > that, I didn't follow), but there's no mmu_notifier to ensure that > > access is synchronized with pte updates. > > > > Hence mark these as unsafe. This means that with > > CONFIG_STRICT_FOLLOW_PFN, these will be rejected. > > > > Real fix is to wire up an mmu_notifier ... somehow. Probably means any > > invalidate is a fatal fault for this vfio device, but then this > > shouldn't ever happen if userspace is reasonable. > > > > Signed-off-by: Daniel Vetter <[email protected]> > > Cc: Jason Gunthorpe <[email protected]> > > Cc: Kees Cook <[email protected]> > > Cc: Dan Williams <[email protected]> > > Cc: Andrew Morton <[email protected]> > > Cc: John Hubbard <[email protected]> > > Cc: Jérôme Glisse <[email protected]> > > Cc: Jan Kara <[email protected]> > > Cc: Dan Williams <[email protected]> > > Cc: [email protected] > > Cc: [email protected] > > Cc: [email protected] > > Cc: [email protected] > > Cc: Alex Williamson <[email protected]> > > Cc: Cornelia Huck <[email protected]> > > Cc: [email protected] > > --- > > drivers/vfio/vfio_iommu_type1.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/vfio/vfio_iommu_type1.c > > b/drivers/vfio/vfio_iommu_type1.c > > index 5fbf0c1f7433..a4d53f3d0a35 100644 > > --- a/drivers/vfio/vfio_iommu_type1.c > > +++ b/drivers/vfio/vfio_iommu_type1.c > > @@ -421,7 +421,7 @@ static int follow_fault_pfn(struct vm_area_struct *vma, > > struct mm_struct *mm, > > { > > int ret; > > > > - ret = follow_pfn(vma, vaddr, pfn); > > + ret = unsafe_follow_pfn(vma, vaddr, pfn); > > if (ret) { > > bool unlocked = false; > > > > @@ -435,7 +435,7 @@ static int follow_fault_pfn(struct vm_area_struct *vma, > > struct mm_struct *mm, > > if (ret) > > return ret; > > > > - ret = follow_pfn(vma, vaddr, pfn); > > + ret = unsafe_follow_pfn(vma, vaddr, pfn); > > } > > This is actually being commonly used, so it needs fixing. > > When I talked to Alex about this last we had worked out a patch series > that adds a test on vm_ops that the vma came from vfio in the first > place. The VMA's created by VFIO are 'safe' as the PTEs are never changed.
Hm, but wouldn't need that the semi-nasty vma_open trick to make sure that vma doesn't untimely disappear? Or is the idea to look up the underlying vfio object, and refcount that directly? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
