Re: [PATCH 2/3] drivers/vfio/pci: Fix wrong MSI interrupt count

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2014-03-03 at 11:24 +0800, Gavin Shan wrote:
> According PCI local bus specification, the register of Message
> Control for MSI (offset: 2, length: 2) has bit#0 to enable or
> disable MSI logic and it shouldn't be part contributing to the
> calculation of MSI interrupt count.
> 
> The patch fixes above issue. Also, the patch renames local variable
> "flags" to "ctl" for both MSI and MSIx case.

I agree with the count fix, but not the variable rename, unless you also
want to rename PCI_MSI_FLAGS to PCI_MSI_CTL.

Looking at the severity of this bug, QEMU never uses the count we report
and while we will let userspace attempt to enable more vectors than are
available on the device, we never let them access data beyond what
actually gets enabled.  So I don't think it has any security
implications.  Good find though!  Thanks,

Alex

> Signed-off-by: Gavin Shan <shangw@xxxxxxxxxxxxxxxxxx>
> ---
>  drivers/vfio/pci/vfio_pci.c |   14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 7ba0424..5760ea6 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -190,25 +190,23 @@ static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type)
>  
>  	} else if (irq_type == VFIO_PCI_MSI_IRQ_INDEX) {
>  		u8 pos;
> -		u16 flags;
> +		u16 ctl;
>  
>  		pos = vdev->pdev->msi_cap;
>  		if (pos) {
>  			pci_read_config_word(vdev->pdev,
> -					     pos + PCI_MSI_FLAGS, &flags);
> -
> -			return 1 << (flags & PCI_MSI_FLAGS_QMASK);
> +					     pos + PCI_MSI_FLAGS, &ctl);
> +			return 1 << ((ctl & PCI_MSI_FLAGS_QMASK) >> 1);
>  		}
>  	} else if (irq_type == VFIO_PCI_MSIX_IRQ_INDEX) {
>  		u8 pos;
> -		u16 flags;
> +		u16 ctl;
>  
>  		pos = vdev->pdev->msix_cap;
>  		if (pos) {
>  			pci_read_config_word(vdev->pdev,
> -					     pos + PCI_MSIX_FLAGS, &flags);
> -
> -			return (flags & PCI_MSIX_FLAGS_QSIZE) + 1;
> +					     pos + PCI_MSIX_FLAGS, &ctl);
> +			return (ctl & PCI_MSIX_FLAGS_QSIZE) + 1;
>  		}
>  	} else if (irq_type == VFIO_PCI_ERR_IRQ_INDEX)
>  		if (pci_is_pcie(vdev->pdev))



--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux