Re: [PATCH v2 0/3] vfio-pci: Block user access to disabled device MMIO

[Alex Williamson]

On Thu, 7 May 2020 17:59:08 -0400
Peter Xu <peterx@xxxxxxxxxx> wrote:

> On Tue, May 05, 2020 at 03:54:36PM -0600, Alex Williamson wrote:
> > v2:
> > 
> > Locking in 3/ is substantially changed to avoid the retry scenario
> > within the fault handler, therefore a caller who does not allow retry
> > will no longer receive a SIGBUS on contention.  IOMMU invalidations
> > are still not included here, I expect that will be a future follow-on
> > change as we're not fundamentally changing that issue in this series.
> > The 'add to vma list only on fault' behavior is also still included
> > here, per the discussion I think it's still a valid approach and has
> > some advantages, particularly in a VM scenario where we potentially
> > defer the mapping until the MMIO BAR is actually DMA mapped into the
> > VM address space (or the guest driver actually accesses the device
> > if that DMA mapping is eliminated at some point).  Further discussion
> > and review appreciated.  Thanks,  
> 
> Hi, Alex,
> 
> I have a general question on the series.
> 
> IIUC this series tries to protect illegal vfio userspace writes to device MMIO
> regions which may cause platform-level issues.  That makes perfect sense to me.
> However what if the write comes from the devices' side?  E.g.:
> 
>   - Device A maps MMIO region X
> 
>   - Device B do VFIO_IOMMU_DMA_MAP on Device A's MMIO region X
>     (so X's MMIO PFNs are mapped in device B's IOMMU page table)
> 
>   - Device A clears PCI_COMMAND_MEMORY (reset, etc.)
>     - this should zap all existing vmas that mapping region X, however device
>       B's IOMMU page table is not aware of this?
> 
>   - Device B writes to MMIO region X of device A even if PCI_COMMAND_MEMORY
>     cleared on device A's PCI_COMMAND register
> 
> Could this happen?

Yes, this can happen and Jason has brought up variations on this
scenario that are important to fix as well.  I've got some ideas, but
the access in this series was the current priority.  There are also
issues in the above scenario that if a platform considers a DMA write
to an invalid IOMMU PTE and triggering an IOMMU fault to have the same
severity as the write to disabled MMIO space we've prevented, then our
hands are tied.  Thanks,

Alex