All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Dimitri Daskalakis" <dimitri.daskalakis1@gmail.com>
Cc: kvm@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [RFC 07/12] PCI: Convert xen-pciback and pci-driver to pci_is_sriov_* helpers
Date: Thu, 04 Jun 2026 15:24:37 +0000	[thread overview]
Message-ID: <20260604152438.510441F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260604150153.3619662-8-dimitri.daskalakis1@gmail.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Incomplete implementation: missed conversion of dev->is_virtfn in vpci.c.

Pre-existing issues:
- [High] Concurrent driver unbind can cause a use-after-free of `pcistub_device` in `pci_stub_notifier`.
--

commit 88d02306e6f1bfe4b75d5df018b4e5de4f2ff35a
Author: Dimitri Daskalakis <daskald@meta.com>

PCI: Convert xen-pciback and pci-driver to pci_is_sriov_* helpers

No functional changes.

Assisted-by: Claude:claude-opus-4.7
Signed-off-by: Dimitri Daskalakis <daskald@meta.com>

[Severity: Medium]
The commit message explicitly states its scope is to convert xen-pciback and
pci-driver to the new pci_is_sriov_* helpers. Did we miss converting
dev->is_virtfn in drivers/xen/xen-pciback/vpci.c?

For example, in __xen_pcibk_add_pci_dev() there are still direct accesses:

drivers/xen/xen-pciback/vpci.c:__xen_pcibk_add_pci_dev() {
    ...
	if (!dev->is_virtfn || func) {
    ...
			if (t->dev->is_virtfn && !PCI_FUNC(t->dev->devfn))
    ...
}

Leaving these direct accesses unconverted means they will fail to distinguish
between SRIOV and SIOV VFs when the helpers are updated to check dev->is_sriov
in later patches.

> diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
> index 79a2b5dfd6941..fd5a13f79fcf4 100644
> --- a/drivers/xen/xen-pciback/pci_stub.c
> +++ b/drivers/xen/xen-pciback/pci_stub.c
> @@ -1695,7 +1695,7 @@ static int pci_stub_notifier(struct notifier_block *nb,
>  	if (action != BUS_NOTIFY_UNBIND_DRIVER)
>  		return NOTIFY_DONE;
>  
> -	if (!pdev->is_physfn)
> +	if (!pci_is_sriov_physfn(pdev))
>  		return NOTIFY_DONE;
>  
>  	for (;;) {

[Severity: High]
This is a pre-existing issue, but is there a use-after-free risk here if a
concurrent unbind occurs?

find_vfs() searches the pcistub_devices list and returns a pcistub_device
pointer without taking a reference count via pcistub_device_get():

drivers/xen/xen-pciback/pci_stub.c:pci_stub_notifier() {
    ...
	for (;;) {
		struct pcistub_device *psdev = find_vfs(pdev);
		if (!psdev)
			break;
		device_release_driver(&psdev->dev->dev);
	}
    ...
}

If a concurrent thread unbinds the VF (e.g., via sysfs unbind),
pcistub_remove() executing under the VF's device lock will remove the device
from the list and drop the final reference using pcistub_device_put(), freeing
psdev before the notifier has a chance to execute.

The notifier thread would then dereference the freed psdev memory when
evaluating &psdev->dev->dev in device_release_driver().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604150153.3619662-1-dimitri.daskalakis1@gmail.com?part=7

  parent reply	other threads:[~2026-06-04 15:24 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-04 15:01 [RFC 00/12] PCI: Add support for Scalable I/O Virtualization Dimitri Daskalakis
2026-06-04 15:01 ` [RFC 01/12] PCI: Add helpers to identify SR-IOV PFs/VFs Dimitri Daskalakis
2026-06-04 15:01 ` [RFC 02/12] PCI: Convert iov.c to pci_is_sriov_* helpers Dimitri Daskalakis
2026-06-04 15:15   ` sashiko-bot
2026-06-04 15:01 ` [RFC 03/12] PCI: Convert pci.h " Dimitri Daskalakis
2026-06-04 15:01 ` [RFC 04/12] PCI: Convert arch/powerpc " Dimitri Daskalakis
2026-06-04 17:26   ` sashiko-bot
2026-06-04 15:01 ` [RFC 05/12] PCI: Convert s390/pci/pci.c " Dimitri Daskalakis
2026-06-04 15:01 ` [RFC 06/12] PCI: Convert vfio_pci_core.c " Dimitri Daskalakis
2026-06-04 15:01 ` [RFC 07/12] PCI: Convert xen-pciback and pci-driver " Dimitri Daskalakis
2026-06-04 15:11   ` Juergen Gross
2026-06-04 15:24   ` sashiko-bot [this message]
2026-06-04 15:01 ` [RFC 08/12] PCI: Add is_sriov bit to struct pci_dev Dimitri Daskalakis
2026-06-04 15:01 ` [RFC 09/12] PCI: Add helper to compute VF Routing ID to pci.h Dimitri Daskalakis
2026-06-04 15:01 ` [RFC 10/12] PCI: Add Scalable I/O Virtualization data structure definitions Dimitri Daskalakis
2026-06-04 15:23   ` sashiko-bot
2026-06-04 15:01 ` [RFC 11/12] PCI: Initialize and release SIOV capability Dimitri Daskalakis
2026-06-04 15:23   ` sashiko-bot
2026-06-04 15:01 ` [RFC 12/12] PCI: Reserve bus range for SIOV devices Dimitri Daskalakis
2026-06-04 15:25   ` sashiko-bot
2026-06-04 18:20 ` [RFC 00/12] PCI: Add support for Scalable I/O Virtualization Jason Gunthorpe
2026-06-04 23:49   ` Dimitri Daskalakis
2026-06-04 23:53     ` Jason Gunthorpe
2026-06-05  0:59     ` Jakub Kicinski
2026-06-05  4:14 ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260604152438.510441F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dimitri.daskalakis1@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.