All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Ziming Du <duziming2@huawei.com>
Cc: bhelgaas@google.com, alex@shazbot.org, chrisw@redhat.com,
	jbarnes@virtuousgeek.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, liuyongqiang13@huawei.com
Subject: Re: [PATCH v4 2/4] PCI/sysfs: Fix null pointer dereference during hotplug
Date: Thu, 26 Feb 2026 11:14:03 -0600	[thread overview]
Message-ID: <20260226171403.GA3813150@bhelgaas> (raw)
In-Reply-To: <20260116081723.1603603-3-duziming2@huawei.com>

On Fri, Jan 16, 2026 at 04:17:19PM +0800, Ziming Du wrote:
> During the concurrent process of creating and rescanning in VF, the
> resource files for the same pci_dev may be created twice.

Where are the two resource file creations?  This will help review the
patch.

> The second
> creation attempt fails, resulting the res_attr in pci_dev to kfree(),
> but the pointer is not set to NULL. This will subsequently lead to
> dereferencing a null pointer when removing the device.
> 
> When we perform the following operation:
>   echo $sriov_totalvfs > /sys/class/net/"$pfname"/device/sriov_numvfs &

I think it would be more informative to include an actual sample here.
We can easily substitute the device names and numbers, given a
concrete example.  It's a little bit harder to intuit what $pfname and
$sriov_totalvfs should be.  E.g.,

  $ cat /sys/bus/pci/devices/0000:02:00.0/sriov_totalvfs
  128
  $ echo 128 > /sys/bus/pci/devices/0000:02:00.0/sriov_numvfs &

Unless it's important to use /sys/class/net/..., use
/sys/bus/pci/devices/... both places to make it simpler.

>   sleep 0.5
>   echo 1 > /sys/bus/pci/rescan

These look like shell commands ...

>   pci_remove "$pfname"

but what is "pci_remove"?  I guess it must be an echo into
/sys/bus/pci/devices/.../remove; expanding it here would be better.

> system will crash as follows:
> 
>   Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
>   Call trace:
>    __pi_strlen+0x14/0x150
>    kernfs_find_ns+0x54/0x120
>    kernfs_remove_by_name_ns+0x58/0xf0
>    sysfs_remove_bin_file+0x24/0x38
>    pci_remove_resource_files+0x44/0x90
>    pci_remove_sysfs_dev_files+0x28/0x40
>    pci_stop_bus_device+0xb8/0x118
>    pci_stop_and_remove_bus_device+0x20/0x40
>    pci_iov_remove_virtfn+0xb8/0x138
>    sriov_disable+0xbc/0x190
>    pci_disable_sriov+0x30/0x48
>    hinic_pci_sriov_disable+0x54/0x138 [hinic]
>    hinic_remove+0x140/0x290 [hinic]
>    pci_device_remove+0x4c/0xf8
>    device_remove+0x54/0x90
>    device_release_driver_internal+0x1d4/0x238
>    device_release_driver+0x20/0x38
>    pci_stop_bus_device+0xa8/0x118
>    pci_stop_and_remove_bus_device_locked+0x28/0x50
>    remove_store+0x128/0x208
> 
> Fix this by set the pointer to NULL after releasing 'res_attr' immediately.

This *sounds* like it would still be racy unless there's a lock around
this.  If there is a lock, please mention what it is and where it's
held.

> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Ziming Du <duziming2@huawei.com>
> ---
>  drivers/pci/pci-sysfs.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 18e5d4603b472..fbcbf39232732 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1227,12 +1227,14 @@ static void pci_remove_resource_files(struct pci_dev *pdev)
>  		if (res_attr) {
>  			sysfs_remove_bin_file(&pdev->dev.kobj, res_attr);
>  			kfree(res_attr);
> +			pdev->res_attr[i] = NULL;
>  		}
>  
>  		res_attr = pdev->res_attr_wc[i];
>  		if (res_attr) {
>  			sysfs_remove_bin_file(&pdev->dev.kobj, res_attr);
>  			kfree(res_attr);
> +			pdev->res_attr_wc[i] = NULL;
>  		}
>  	}
>  }
> -- 
> 2.43.0
> 

  reply	other threads:[~2026-02-26 17:14 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-16  8:17 [PATCH v4 0/4] Miscellaneous fixes for pci subsystem Ziming Du
2026-01-16  8:17 ` [PATCH v4 1/4] PCI/sysfs: Prohibit unaligned access to I/O port Ziming Du
2026-02-26 17:00   ` Bjorn Helgaas
2026-01-16  8:17 ` [PATCH v4 2/4] PCI/sysfs: Fix null pointer dereference during hotplug Ziming Du
2026-02-26 17:14   ` Bjorn Helgaas [this message]
2026-02-27  2:30     ` duziming
2026-04-02  7:23     ` duziming
2026-05-02 16:24       ` Krzysztof Wilczyński
2026-01-16  8:17 ` [PATCH v4 3/4] PCI: Prevent overflow in proc_bus_pci_write() Ziming Du
2026-03-03 19:32   ` Bjorn Helgaas
2026-01-16  8:17 ` [PATCH v4 4/4] PCI: Prevent overflow in proc_bus_pci_read() Ziming Du
2026-01-30  7:53 ` [PATCH v4 0/4] Miscellaneous fixes for pci subsystem duziming
2026-02-06 22:29   ` Bjorn Helgaas
2026-02-26  9:07     ` duziming

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=20260226171403.GA3813150@bhelgaas \
    --to=helgaas@kernel.org \
    --cc=alex@shazbot.org \
    --cc=bhelgaas@google.com \
    --cc=chrisw@redhat.com \
    --cc=duziming2@huawei.com \
    --cc=jbarnes@virtuousgeek.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=liuyongqiang13@huawei.com \
    /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.