Hi Julien, > On Sep 15, 2023, at 20:52, Julien Grall <[email protected]> wrote: > > From: Julien Grall <[email protected]> > > Currently, libxl will grant IOMEM, I/O port and IRQ permissions when > a PCI is attached (see pci_add_dm_done()) for all domain types. However, > the permissions are only revoked for non-HVM domain (see do_pci_remove()). > > This means that HVM domains will be left with extra permissions. While > this look bad on the paper, the IRQ permissions should be revoked > when the Device Model call xc_physdev_unmap_pirq() and such domain > cannot directly mapped I/O port and IOMEM regions. Instead, this has to > be done by a Device Model. > > The Device Model can only run in dom0 or PV stubdomain (upstream libxl > doesn't have support for HVM/PVH stubdomain). > > For PV/PVH stubdomain, the permission are properly revoked, so there is > no security concern. > > This leaves dom0. There are two cases: > 1) Privileged: Anyone gaining access to the Device Model would already > have large control on the host. > 2) Deprivileged: PCI passthrough require PHYSDEV operations which > are not accessible when the Device Model is restricted. > > So overall, it is believed that the extra permissions cannot be exploited. > > Rework the code so the permissions are all removed for HVM domains. > This needs to happen after the QEMU has detached the device. So > the revocation is now moved to pci_remove_detached(). > > Also add a comment on top of the error message when the PIRQ cannot > be unbind to explain this could be a spurious error as QEMU may have > already done it. > > Signed-off-by: Julien Grall <[email protected]>
As in discussion in v1, it is agreed that this patch should be included in 4.18, although technically my release-ack tag should be effective after code freeze, I am still providing the tag to avoid possible confusion: Release-acked-by: Henry Wang <[email protected]> Kind regards, Henry
