linux-kernel - RE: [PATCH] PCI: hv: Fix hv_arch_irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <PH0PR21MB3025DB8EAB4714E059CAC326D7FA9@PH0PR21MB3025.namprd21.prod.outlook.com>
Date:   Wed, 27 Apr 2022 17:11:39 +0000
From:   "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To:     Jeffrey Hugo <quic_jhugo@...cinc.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        Dexuan Cui <decui@...rosoft.com>,
        "lorenzo.pieralisi@....com" <lorenzo.pieralisi@....com>,
        "robh@...nel.org" <robh@...nel.org>, "kw@...ux.com" <kw@...ux.com>,
        "bhelgaas@...gle.com" <bhelgaas@...gle.com>
CC:     "bjorn.andersson@...aro.org" <bjorn.andersson@...aro.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] PCI: hv: Fix hv_arch_irq_unmask() for multi-MSI

From: Jeffrey Hugo <quic_jhugo@...cinc.com> Sent: Wednesday, April 27, 2022 7:08 AM
> 
> In the multi-MSI case, hv_arch_irq_unmask() will only operate on the first
> MSI of the N allocated.  This is because only the first msi_desc is cached
> and it is shared by all the MSIs of the multi-MSI block.  This means that
> hv_arch_irq_unmask() gets the correct address, but the wrong data (always
> 0).
> 
> This can break MSIs.
> 
> Lets assume MSI0 is vector 34 on CPU0, and MSI1 is vector 33 on CPU0.
> 
> hv_arch_irq_unmask() is called on MSI0.  It uses a hypercall to configure
> the MSI address and data (0) to vector 34 of CPU0.  This is correct.  Then
> hv_arch_irq_unmask is called on MSI1.  It uses another hypercall to
> configure the MSI address and data (0) to vector 33 of CPU0.  This is
> wrong, and results in both MSI0 and MSI1 being routed to vector 33.  Linux
> will observe extra instances of MSI1 and no instances of MSI0 despite the
> endpoint device behaving correctly.
> 
> For the multi-MSI case, we need unique address and data info for each MSI,
> but the cached msi_desc does not provide that.  However, that information
> can be gotten from the int_desc cached in the chip_data by
> compose_msi_msg().  Fix the multi-MSI case to use that cached information
> instead.  Since hv_set_msi_entry_from_desc() is no longer applicable,
> remove it.
> 
> Signed-off-by: Jeffrey Hugo <quic_jhugo@...cinc.com>
> ---
>  drivers/pci/controller/pci-hyperv.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 5800ecf..7aea0b7 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -611,13 +611,6 @@ static unsigned int hv_msi_get_int_vector(struct irq_data
> *data)
>  	return cfg->vector;
>  }
> 
> -static void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry,
> -				       struct msi_desc *msi_desc)
> -{
> -	msi_entry->address.as_uint32 = msi_desc->msg.address_lo;
> -	msi_entry->data.as_uint32 = msi_desc->msg.data;
> -}
> -
>  static int hv_msi_prepare(struct irq_domain *domain, struct device *dev,
>  			  int nvec, msi_alloc_info_t *info)
>  {
> @@ -647,6 +640,7 @@ static void hv_arch_irq_unmask(struct irq_data *data)
>  {
>  	struct msi_desc *msi_desc = irq_data_get_msi_desc(data);
>  	struct hv_retarget_device_interrupt *params;
> +	struct tran_int_desc *int_desc;
>  	struct hv_pcibus_device *hbus;
>  	struct cpumask *dest;
>  	cpumask_var_t tmp;
> @@ -661,6 +655,7 @@ static void hv_arch_irq_unmask(struct irq_data *data)
>  	pdev = msi_desc_to_pci_dev(msi_desc);
>  	pbus = pdev->bus;
>  	hbus = container_of(pbus->sysdata, struct hv_pcibus_device, sysdata);
> +	int_desc = data->chip_data;
> 
>  	spin_lock_irqsave(&hbus->retarget_msi_interrupt_lock, flags);
> 
> @@ -668,7 +663,8 @@ static void hv_arch_irq_unmask(struct irq_data *data)
>  	memset(params, 0, sizeof(*params));
>  	params->partition_id = HV_PARTITION_ID_SELF;
>  	params->int_entry.source = HV_INTERRUPT_SOURCE_MSI;
> -	hv_set_msi_entry_from_desc(&params->int_entry.msi_entry, msi_desc);
> +	params->int_entry.msi_entry.address.as_uint32 = int_desc->address &
> 0xffffffff;
> +	params->int_entry.msi_entry.data.as_uint32 = int_desc->data;
>  	params->device_id = (hbus->hdev->dev_instance.b[5] << 24) |
>  			   (hbus->hdev->dev_instance.b[4] << 16) |
>  			   (hbus->hdev->dev_instance.b[7] << 8) |
> --
> 2.7.4

Reviewed-by: Michael Kelley <mikelley@...rosoft.com>