[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v5 1/3] x86/msi: harden stale pdev handling
On 10/15/24 02:58, Jan Beulich wrote: > On 11.10.2024 17:27, Stewart Hildebrand wrote: >> --- a/xen/arch/x86/msi.c >> +++ b/xen/arch/x86/msi.c >> @@ -1243,7 +1243,12 @@ int pci_reset_msix_state(struct pci_dev *pdev) >> { >> unsigned int pos = pci_find_cap_offset(pdev->sbdf, PCI_CAP_ID_MSIX); >> >> - ASSERT(pos); >> + if ( !pos ) >> + { >> + pdev->broken = true; >> + return -EFAULT; >> + } >> + >> /* >> * Xen expects the device state to be the after reset one, and hence >> * host_maskall = guest_maskall = false and all entries should have the >> @@ -1271,7 +1276,12 @@ int pci_msi_conf_write_intercept(struct pci_dev >> *pdev, unsigned int reg, >> entry = find_msi_entry(pdev, -1, PCI_CAP_ID_MSIX); >> pos = entry ? entry->msi_attrib.pos >> : pci_find_cap_offset(pdev->sbdf, PCI_CAP_ID_MSIX); >> - ASSERT(pos); >> + >> + if ( !pos ) >> + { >> + pdev->broken = true; >> + return -EFAULT; >> + } >> >> if ( reg >= pos && reg < msix_pba_offset_reg(pos) + 4 ) >> { > > There are more instances of pci_find_cap_offset(..., PCI_CAP_ID_MSIX) > which may want/need dealing with, even if there are no ASSERT()s there. Yes, and some instances of pci_find_cap_offset(..., PCI_CAP_ID_MSI) too. > Setting ->broken is of course a perhaps desirable (side) effect. Nevertheless > I wonder whether latching the capability position once during device init > wouldn't be an alternative (better?) approach. I'll give this a try for the next rev. > Finally I don't think -EFAULT is appropriate here. Imo it should be -ENODEV. OK > > Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |