Re: [Xen-devel] [PATCH] x86/msi: Validate the guest-identified PCI devices in pci_prepare_msix()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.01.14 at 11:28, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> On 22/01/14 09:49, Jan Beulich wrote:
>>>>> On 22.01.14 at 05:31, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> 
>>>>> wrote:
>>> See attached (and relevant part inlined).
>>> ...
>>> (XEN) [2014-01-22 12:27:07] Xen call trace:
>>> (XEN) [2014-01-22 12:27:07]    [<ffff82d0801683a2>] 
> msix_capability_init+0x1dc/0x603
>>> (XEN) [2014-01-22 12:27:07]    [<ffff82d080168987>] 
>>> pci_enable_msi+0x1be/0x4d7
>>> (XEN) [2014-01-22 12:27:07]    [<ffff82d08016c65c>] 
>>> map_domain_pirq+0x222/0x5ad
>>> (XEN) [2014-01-22 12:27:07]    [<ffff82d08017f104>] 
>>> physdev_map_pirq+0x507/0x5d1
>>> (XEN) [2014-01-22 12:27:07]    [<ffff82d08017f814>] 
>>> do_physdev_op+0x646/0x119e
>>> (XEN) [2014-01-22 12:27:07]    [<ffff82d08022231b>] syscall_enter+0xeb/0x145
>>> (XEN) [2014-01-22 12:27:07] 
>>> (XEN) [2014-01-22 12:27:07] Pagetable walk from 0000000000000004:
>> Considering the similarity, this is surely another incarnation of
>> the same issue. Which gets me to ask first of all - is the device
>> being acted upon an MSI-X capable one? If not, why is the call
>> being made? If so (and Xen thinks differently) that's what
>> needs fixing.
>>
>> On that basis I'm also going to ignore your patch for the first
>> problem, Andrew: It's either incomplete or unnecessary or
>> fixing the wrong thing.
> 
> I am going to go with incomplete - it is certainly not unnecessary.  The
> PCI device parameters to pci_prepare_msix() are completely guest
> controlled; There is no validation of the SBDF at all.

"Fixing the wrong thing" presumably, after taking a closer look at
Konrad's second crash: The device in question really appears to
be MSI-X capable, yet alloc_pdev() didn't recognize it as such. I
wonder whether the capability gets displayed/hidden dynamically
based on some other enabling the driver may be doing on the
device. In which case we'd need to allocate the structure on
demand.

But of course I'd like to first have confirmation that that's really
what is happening here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel