RE: [PATCH 5/5] vfio-pci: Allow to mmap MSI-X table if interrupt remapping is supported
From: "Tian, Kevin" <kevin.tian@intel.com>
Date: 2016-05-13 06:50:32
Also in:
kvm, linux-iommu, linux-pci, lkml
From: Alex Williamson [mailto:alex.williamson@redhat.com] Sent: Friday, May 13, 2016 1:33 PMquoted
quoted
As argued previously in this thread, there's nothing special about a DMA write to memory versus a DMA write to a special address that triggers an MSI vector. If the device is DMA capable, which we assum=
e
quoted
quoted
all are, it can be fooled into generating those DMA writes regardless of whether we actively block access to the MSI-X vector table itself.But with DMA remapping above can be blocked.=20 How? VT-d explicitly ignores DMA writes to 0xFEEx_xxxx, section 3.13: =20 Write requests without PASID of DWORD length are treated as interrupt requests. Interrupt requests are not subjected to DMA remapping[...] Instead, remapping hardware can be enabled to subject such interrupt requests to interrupt remapping.
Thanks for catching this!
=20quoted
quoted
MSI-X vector table access w/o interrupt remapping is to avoid obvious collisions if it were to be programmed directly, it doesn't actually prevent an identical DMA transaction from being generated by otherKernel can enable DMA remapping but disable IRQ remapping. In such case identical DMA transaction can be prevented.=20 Not according to the VT-d spec as quoted above. If so, how?
So my argument on this is wrong. sorry.
=20quoted
Anyway my point is simple. Let's ignore how Linux kernel implements IRQ remapping on x86 (which may change time to time), and just focus on architectural possibility. Non-x86 platform may implement IRQ remapping completely separate from device side, then checking availability of IRQ remapping is enough to decide whether mmap MSI-X table is safe. x86 with VT-d can be configured to a mode requiring host control of both MSI-X entry and IRQ remapping hardware (without source id check). In such case it's insufficient to make decision simply based on IRQ remapping availability. We need a way to query from IRQ remapping module whether it's actually safe to mmap MSI-X.=20 We're going in circles here. This patch is attempting to remove protection from the MSI-X vector table that is really nothing more than security theater already. That "protection" only actually prevents casual misuse of the API which is really only a problem when the platform offers no form of interrupt isolation, such as VT-d with DMA remapping but not interrupt remapping. Disabling source-id checking in VT-d should be handled as the equivalent of disabling interrupt remapping altogether as far as the IOMMU API is concerned. That's a trivial gap that should be fixed. There is no such thing as a secure
That is the main change I'm asking against original patch, which has:
+static void pci_check_msi_remapping(struct pci_dev *pdev,
+ const struct iommu_ops *ops)
+{
+ struct pci_bus *bus =3D pdev->bus;
+
+ if (ops->capable(IOMMU_CAP_INTR_REMAP) &&
+ !(bus->bus_flags & PCI_BUS_FLAGS_MSI_REMAP))
+ bus->bus_flags |=3D PCI_BUS_FLAGS_MSI_REMAP;
+}
+
Above flag should be cleared when source-id checking is disabled on x86.=20
Yes, VFIO is part of OS but any assumption we made about other parts
needs to be reflected accurately in the code.
MSI-X vector table when untrusted userspace drivers are involved, we must always assume that a device can generate DMA writes that are indistinguishable from actual interrupt requests and if the platform does not provide interrupt isolation we should require the user to opt-in to an unsafe mode. =20 Simply denying direct writes to the vector table or preventing mapping of the vector table into the user address space does not provide any tangible form of protection. Many devices make use of window registers that allow backdoors to arbitrary device registers. Some drivers even use this as the primary means for configuring MSI-X, which makes them incompatible with device assignment without device specific quirks to enable virtualization of these paths. =20 If you have an objection to this patch, please show me how preventing direct CPU access to the MSI-X vector table provides any kind of security guarantee of the contents of the vector table and also prove to me that a device cannot spoof a DMA write that is indistinguishable from one associated with an actual interrupt, regardless of the contents of the MSI-X vector table. Thanks, =20
I'm not object to the whole patch series. As replied above, my point is just that current condition of allowing mmap MSI-X in this patch is not= =20 accurate, but my argument on security manner is not correct. Thanks for your elaboration to make it clear. Thanks Kevin