linux-kernel - RE: [PATCH v3 6/6] PCI: hv: fix 2 hang issues in hv_compose_msi

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:   Wed, 7 Mar 2018 21:40:03 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>
CC:     "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        KY Srinivasan <kys@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "olaf@...fle.de" <olaf@...fle.de>,
        "apw@...onical.com" <apw@...onical.com>,
        "jasowang@...hat.com" <jasowang@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "driverdev-devel@...uxdriverproject.org" 
        <driverdev-devel@...uxdriverproject.org>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        "vkuznets@...hat.com" <vkuznets@...hat.com>,
        "marcelo.cerri@...onical.com" <marcelo.cerri@...onical.com>,
        "Michael Kelley (EOSG)" <Michael.H.Kelley@...rosoft.com>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Jack Morgenstein <jackm@...lanox.com>
Subject: RE: [PATCH v3 6/6] PCI: hv: fix 2 hang issues in hv_compose_msi_msg()

> From: Lorenzo Pieralisi <lorenzo.pieralisi@....com>
> Sent: Wednesday, March 7, 2018 04:35
> On Tue, Mar 06, 2018 at 06:21:56PM +0000, Dexuan Cui wrote:
> > 1. With the patch "x86/vector/msi: Switch to global reservation mode"
> > (4900be8360), the recent v4.15 and newer kernels always hang for 1-vCPU
> > Hyper-V VM with SR-IOV. This is because when we reach
> hv_compose_msi_msg()
> > by request_irq()  -> request_threaded_irq() -> __setup_irq()->irq_startup()
> >  -> __irq_startup() -> irq_domain_activate_irq() -> ... ->
> > msi_domain_activate() -> ... -> hv_compose_msi_msg(), local irq is
> > disabled in __setup_irq().
> >
> > Fix this by polling the channel.
> >
> > 2. If the host is ejecting the VF device before we reach
> > hv_compose_msi_msg(), in a UP VM, we can hang in hv_compose_msi_msg()
> > forever, because at this time the host doesn't respond to the
> > CREATE_INTERRUPT request. This issue also happens to old kernels like
> > v4.14, v4.13, etc.
>
> If you are fixing a problem you should report what commit you are fixing
> with a Fixes: tag and add a CC: stable@...r.kernel.org to the commit log
> to send it to stable kernels to which it should be applied; mentioning
> kernel versions in the commit log is useless and should be omitted.

Hi Lorenzo,
Thanks for your comments!
This patch does have a "Cc: stable@...r.kernel.org" in the sign-off area. :-)

Here the patch is made to resolve 2 issues:
#1 is triggered by the x86 global reservation mode (4900be8360) patch.
4900be8360 in itself is good. It's just that drivers/pci/host/pci-hyperv.c
should be fixed.

#2 is a longstanding issue since the first day the pci-hyperv driver was
accepted into the kernel.

So IMO actually we don't really need to add a Fixes: tag, which is usually
used to specify a specific commit that introduces a bug that is being fixed.

> Side note: you should not have stable@...r.kernel.org in the email
> addresses CC list you are sending the patches to (you mark patches for
> stable by adding an appropriate CC tag in the commit log).

Sorry, I didn't know this, but actually I didn't add stable@...r.kernel.org
manually. Instead I used "git send-email" to send this patchset, and it told
me "The Cc list above has been expanded by additional addresses found
in the patch commit message."

I didn't find a way to disable this behavior of "git send-email" by checking
its manual and googling it. This is strange.

> Here:
>
> git.kernel.org/.../Documentation/process/stable-kernel-rules.rst
>
> Last but not least, most of the patches in this series do not justify
> sending them to stable kernels at all so you should remove the
> corresponding tag from the patches.

I hope at least these 2 patches can go into the stable kernels:
[PATCH v3 3/6] PCI: hv: serialize the present/eject work items
[PATCH v3 6/6] PCI: hv: fix 2 hang issues in hv_compose_msi_msg()
Especially the second one, which fixes a real hang issue for UP virtual
machines running v4.15 and newer.
And,  IMO the patches are small enough (<100 lines) , but definitely
the maintainers make the final call.

>
> Thanks,
> Lorenzo
>
> > Fix this by polling the channel for the PCI_EJECT message and
> > hpdev->state, and by checking the PCI vendor ID.
> >
> > Note: actually the above issues also happen to a SMP VM, if
> > "hbus->hdev->channel->target_cpu == smp_processor_id()" is true.
> >
> > Signed-off-by: Dexuan Cui <decui@...rosoft.com>
> > Tested-by: Adrian Suhov <v-adsuho@...rosoft.com>
> > Tested-by: Chris Valean <v-chvale@...rosoft.com>
> > Cc: stable@...r.kernel.org
> > Cc: Stephen Hemminger <sthemmin@...rosoft.com>
> > Cc: K. Y. Srinivasan <kys@...rosoft.com>
> > Cc: Vitaly Kuznetsov <vkuznets@...hat.com>
> > Cc: Jack Morgenstein <jackm@...lanox.com>
> > ---
> >  drivers/pci/host/pci-hyperv.c | 58


Thanks,
-- Dexuan