Re: [Xen-devel] [PATCH] msi: Avoid uninitialized msi descriptors

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> On 11.08.10 at 15:43, Wei Wang2 <wei.wang2@xxxxxxx> wrote:
> static int __pci_enable_msi(struct msi_info *msi, struct msi_desc **desc)
> {
>     int status;
>     struct pci_dev *pdev;
>+    struct msi_desc *old_desc;
> 
>     ASSERT(spin_is_locked(&pcidevs_lock));
>     pdev = pci_get_pdev(msi->bus, msi->devfn);
>     if ( !pdev )
>         return -ENODEV;
> 
>-    if ( find_msi_entry(pdev, msi->irq, PCI_CAP_ID_MSI) )
>+    old_desc = find_msi_entry(pdev, msi->irq, PCI_CAP_ID_MSI);
>+    if ( old_desc )
>     {
>         dprintk(XENLOG_WARNING, "irq %d has already mapped to MSI on "
>                 "device %02x:%02x.%01x.\n", msi->irq, msi->bus,
>                 PCI_SLOT(msi->devfn), PCI_FUNC(msi->devfn));
>+        *desc = old_desc;

While I agree to this part, ...

>         return 0;
>     }
> 
>-    if ( find_msi_entry(pdev, -1, PCI_CAP_ID_MSIX) )
>+    old_desc = find_msi_entry(pdev, -1, PCI_CAP_ID_MSIX);
>+    if ( old_desc )
>     {
>         dprintk(XENLOG_WARNING, "MSI-X is already in use on "
>                 "device %02x:%02x.%01x\n", msi->bus,
>                 PCI_SLOT(msi->devfn), PCI_FUNC(msi->devfn));
>-        return 0;
>+        pci_disable_msi(old_desc);

... I don't think this one's right: Admittedly I should have changed
the return value from 0 to an actual error (e.g. -EBUSY) - I simply
overlooked that after doing the copy-and-paste operation.

Whether disabling and switching to the alternative mechanism
is the right thing to do here I don't know. But I'm pretty certain
that old_desc may now be leaked, as msi_free_irq() isn't being
called on it and set_irq_msi() also doesn't check whether
irq_desc[].msi_desc is already non-NULL.

Same thing (obviously) for the second part of the changes.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel