On Wed, Oct 7, 2020 at 7:39 PM Jason Gunthorpe <j...@ziepe.ca> 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 <daniel.vet...@intel.com> > > Cc: Jason Gunthorpe <j...@ziepe.ca> > > Cc: Kees Cook <keesc...@chromium.org> > > Cc: Dan Williams <dan.j.willi...@intel.com> > > Cc: Andrew Morton <a...@linux-foundation.org> > > Cc: John Hubbard <jhubb...@nvidia.com> > > Cc: Jérôme Glisse <jgli...@redhat.com> > > Cc: Jan Kara <j...@suse.cz> > > Cc: Dan Williams <dan.j.willi...@intel.com> > > Cc: linux...@kvack.org > > Cc: linux-arm-ker...@lists.infradead.org > > Cc: linux-samsung-...@vger.kernel.org > > Cc: linux-me...@vger.kernel.org > > Cc: Alex Williamson <alex.william...@redhat.com> > > Cc: Cornelia Huck <coh...@redhat.com> > > Cc: k...@vger.kernel.org > > --- > > 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