linux-kernel - RE: [PATCH] PCI: hv: Fix a crash in hv_pci_restore_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]

Message-ID: <BYAPR21MB1688E86B4DB69DB8F3796DDED715A@BYAPR21MB1688.namprd21.prod.outlook.com>
Date:   Wed, 16 Aug 2023 00:35:18 +0000
From:   "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To:     Dexuan Cui <decui@...rosoft.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "jgg@...pe.ca" <jgg@...pe.ca>,
        "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        "kw@...ux.com" <kw@...ux.com>, KY Srinivasan <kys@...rosoft.com>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "lpieralisi@...nel.org" <lpieralisi@...nel.org>,
        "robh@...nel.org" <robh@...nel.org>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        "helgaas@...nel.org" <helgaas@...nel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] PCI: hv: Fix a crash in hv_pci_restore_msi_msg() during
 hibernation

From: Dexuan Cui <decui@...rosoft.com> Sent: Saturday, August 12, 2023 5:12 PM
> 
> For a Linux VM with a NVIDIA GPU running on Hyper-V, before the GPU driver
> is installed, hibernating the VM will trigger a panic: if the GPU driver
> is not installed and loaded, MSI-X/MSI is not enabled on the device, so
> pdev->dev.msi.data is NULL, and msi_lock_descs(&pdev->dev) causes the
> NULL pointer dereference. Fix this by checking pdev->dev.msi.data.

Is the scenario here a little broader than just the NVIDIA GPU driver?  For
any virtual PCI device that is presented in the guest VM as a VMBus device,
the driver might not be installed.  There could have been some initial
problem getting the driver installed, or it might have been manually
uninstalled later in the life of the VM.  Also the host might have rescinded
the virtual PCI device and added it back later, creating another opportunity
where the driver might not be loaded.  In any case, it seems like we could
have the VMBus aspects of the device setup, but not the driver for the
device.  This suspend/resume code in pci-hyperv.c is all about handling
the VMBus aspects of the device anyway.

Assuming my thinking is correct, is there some Hyper-V/VMBus setting
owned by the pci-hyperv.c driver that would be better to test here than
the low-level dev.msi.data pointer?  The MSI code rework that added
the descriptor lock encapsulates the internals with appropriate accessor
functions, and reaching in to directly test dev.msi.data violates that
encapsulation.

That said, I think this code works as written.  I'm picking at details.

Michael

> 
> Fixes: dc2b453290c4 ("PCI: hv: Rework MSI handling")
> Signed-off-by: Dexuan Cui <decui@...rosoft.com>
> ---
>  drivers/pci/controller/pci-hyperv.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 2d93d0c4f10d..fdd01bfb8e10 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -3983,6 +3983,9 @@ static int hv_pci_restore_msi_msg(struct pci_dev *pdev,
> void *arg)
>  	struct msi_desc *entry;
>  	int ret = 0;
> 
> +	if (!pdev->dev.msi.data)
> +		return 0;
> +
>  	msi_lock_descs(&pdev->dev);
>  	msi_for_each_desc(entry, &pdev->dev, MSI_DESC_ASSOCIATED) {
>  		irq_data = irq_get_irq_data(entry->irq);
> --
> 2.25.1