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]

Date:   Thu, 28 Apr 2022 15:06:53 +0000
From:   Wei Liu <wei.liu@...nel.org>
To:     Jeffrey Hugo <quic_jhugo@...cinc.com>
Cc:     kys@...rosoft.com, haiyangz@...rosoft.com, sthemmin@...rosoft.com,
        wei.liu@...nel.org, decui@...rosoft.com, lorenzo.pieralisi@....com,
        robh@...nel.org, kw@...ux.com, bhelgaas@...gle.com,
        bjorn.andersson@...aro.org, linux-hyperv@...r.kernel.org,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI: hv: Fix hv_arch_irq_unmask() for multi-MSI

On Thu, Apr 28, 2022 at 02:58:24PM +0000, Wei Liu wrote:
> On Wed, Apr 27, 2022 at 08:07:33AM -0600, Jeffrey Hugo wrote:
> > 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;
> > -}
> > -
> 
> Instead of dropping this function, can you change the second argument to
> take struct tran_int_desc *?
> 
> This way you can use the same function in hv_compose_msi_msg.
> 

Never mind. I think I mixed things up. Sorry for the noise.