linux-kernel - Re: [PATCH v2] pci: fix device presence detection for VFs

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:   Fri, 11 Nov 2022 17:39:07 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     "Michael S. Tsirkin" <mst@...hat.com>
Cc:     linux-kernel@...r.kernel.org, Wei Gong <gongwei833x@...il.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org
Subject: Re: [PATCH v2] pci: fix device presence detection for VFs

On Wed, Oct 26, 2022 at 02:11:21AM -0400, Michael S. Tsirkin wrote:
> virtio uses the same driver for VFs and PFs.  Accordingly,
> pci_device_is_present is used to detect device presence. This function
> isn't currently working properly for VFs since it attempts reading
> device and vendor ID. As VFs are present if and only if PF is present,
> just return the value for that device.
> 
> Reported-by: Wei Gong <gongwei833x@...il.com>
> Signed-off-by: Michael S. Tsirkin <mst@...hat.com>

Applied as below to pci/enumeration for v6.2, thanks!

commit 98b04dd0b457 ("PCI: Fix pci_device_is_present() for VFs by checking PF")
Author: Michael S. Tsirkin <mst@...hat.com>
Date:   Wed Oct 26 02:11:21 2022 -0400

    PCI: Fix pci_device_is_present() for VFs by checking PF
    
    pci_device_is_present() previously didn't work for VFs because it reads the
    Vendor and Device ID, which are 0xffff for VFs, which looks like they
    aren't present.  Check the PF instead.
    
    Wei Gong reported that if virtio I/O is in progress when the driver is
    unbound or "0" is written to /sys/.../sriov_numvfs, the virtio I/O
    operation hangs, which may result in output like this:
    
      task:bash state:D stack:    0 pid: 1773 ppid:  1241 flags:0x00004002
      Call Trace:
       schedule+0x4f/0xc0
       blk_mq_freeze_queue_wait+0x69/0xa0
       blk_mq_freeze_queue+0x1b/0x20
       blk_cleanup_queue+0x3d/0xd0
       virtblk_remove+0x3c/0xb0 [virtio_blk]
       virtio_dev_remove+0x4b/0x80
       ...
       device_unregister+0x1b/0x60
       unregister_virtio_device+0x18/0x30
       virtio_pci_remove+0x41/0x80
       pci_device_remove+0x3e/0xb0
    
    This happened because pci_device_is_present(VF) returned "false" in
    virtio_pci_remove(), so it called virtio_break_device().  The broken vq
    meant that vring_interrupt() skipped the vq.callback() that would have
    completed the virtio I/O operation via virtblk_done().
    
    [bhelgaas: commit log, simplify to always use pci_physfn(), add stable tag]
    Link: https://lore.kernel.org/r/20221026060912.173250-1-mst@redhat.com
    Reported-by: Wei Gong <gongwei833x@...il.com>
    Tested-by: Wei Gong <gongwei833x@...il.com>
    Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@...gle.com>
    Cc: stable@...r.kernel.org

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 9f3cc829dfee..fba95486caaf 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6447,6 +6447,8 @@ bool pci_device_is_present(struct pci_dev *pdev)
 {
 	u32 v;
 
+	/* Check PF if pdev is a VF, since VF Vendor/Device IDs are 0xffff */
+	pdev = pci_physfn(pdev);
 	if (pci_dev_is_disconnected(pdev))
 		return false;
 	return pci_bus_read_dev_vendor_id(pdev->bus, pdev->devfn, &v, 0);