On Thu, Feb 25, 2021 at 02:19:45PM -0400, Jason Gunthorpe wrote: > On Thu, Feb 25, 2021 at 12:54:57PM -0500, Peter Xu wrote: > > > I can't say I fully understand the whole rational behind 5cbf3264bc71, but that > > commit still sounds reasonable to me, since I don't see why VFIO cannot do > > VFIO_IOMMU_MAP_DMA upon another memory range that's neither anonymous memory > > nor vfio mapped MMIO range. > > It is not so much it can't, more that it doesn't and doesn't need to. OK. > > > In those cases, vm_pgoff namespace defined by vfio may not be true > > anymore, iiuc. > > Since this series is proposing linking the VMA to an address_space all > the vm_pgoffs must be in the same namespace Agreed. I saw discussions around on redefining the vm_pgoff namespace, I can't say I followed that closely either, but yes it definitely makes sense to always use an unified namespace. Maybe we should even comment it somewhere on how vm_pgoff is encoded? > > > Or does it mean that we don't want to allow VFIO dma to those unknown memory > > backends, for some reason? > > Correct. VFIO can map into the IOMMU PFNs it can get a reference > to. pin_user_pages() works for the majority, special VFIO VMAs cover > the rest, and everthing else must be blocked for security. If we all agree that the current follow_pfn() should only apply to vfio internal vmas, then it seems we can drop it indeed, as long as the crash reported in 5cbf3264b would fail gracefully at e.g. VFIO_IOMMU_MAP_DMA rather than triggering a kernel warning somehow. However I'm still confused on why it's more secure - the current process to do VFIO_IOMMU_MAP_DMA should at least has proper permission for everything to be setup, including the special vma, right? Say, if the process can write to those memories, then shouldn't we also allow it to grant this write permission to other devices too? Thanks, -- Peter Xu
