Thread (38 messages) 38 messages, 6 authors, 2016-05-25

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 PM
quoted
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!
=20
quoted
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 other
Kernel 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.
=20
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help