Commit 23066026 authored by Konrad Rzeszutek Wilk's avatar Konrad Rzeszutek Wilk Committed by Luis Henriques

xen/pciback: Return error on XEN_PCI_OP_enable_msix when device has MSI or MSI-X enabled

commit 5e0ce145 upstream.

The guest sequence of:

  a) XEN_PCI_OP_enable_msix
  b) XEN_PCI_OP_enable_msix

results in hitting an NULL pointer due to using freed pointers.

The device passed in the guest MUST have MSI-X capability.

The a) constructs and SysFS representation of MSI and MSI groups.
The b) adds a second set of them but adding in to SysFS fails (duplicate entry).
'populate_msi_sysfs' frees the newly allocated msi_irq_groups (note that
in a) pdev->msi_irq_groups is still set) and also free's ALL of the
MSI-X entries of the device (the ones allocated in step a) and b)).

The unwind code: 'free_msi_irqs' deletes all the entries and tries to
delete the pdev->msi_irq_groups (which hasn't been set to NULL).
However the pointers in the SysFS are already freed and we hit an
NULL pointer further on when 'strlen' is attempted on a freed pointer.

The patch adds a simple check in the XEN_PCI_OP_enable_msix to guard
against that. The check for msi_enabled is not stricly neccessary.

This is part of XSA-157
Reviewed-by: default avatarDavid Vrabel <david.vrabel@citrix.com>
Reviewed-by: default avatarJan Beulich <jbeulich@suse.com>
Signed-off-by: default avatarKonrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: default avatarLuis Henriques <luis.henriques@canonical.com>
parent cb968d81
...@@ -206,9 +206,16 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev, ...@@ -206,9 +206,16 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev,
if (unlikely(verbose_request)) if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: enable MSI-X\n", printk(KERN_DEBUG DRV_NAME ": %s: enable MSI-X\n",
pci_name(dev)); pci_name(dev));
if (op->value > SH_INFO_MAX_VEC) if (op->value > SH_INFO_MAX_VEC)
return -EINVAL; return -EINVAL;
if (dev->msix_enabled)
return -EALREADY;
if (dev->msi_enabled)
return -ENXIO;
entries = kmalloc(op->value * sizeof(*entries), GFP_KERNEL); entries = kmalloc(op->value * sizeof(*entries), GFP_KERNEL);
if (entries == NULL) if (entries == NULL)
return -ENOMEM; return -ENOMEM;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment