Re: [PATCH v2 2/2] PCI: Free the device name on the pci_setup_device() error path

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Memory allocated by early fixups (e.g., `dev->saved_cap_space` and `dev->dma_alias_mask`) is leaked when `pci_setup_device()` encounters an unknown header type.
--

--- Patch [2]: [PATCH v2 2/2] PCI: Free the device name on the pci_setup_device() error path ---
commit 25426f7c4f229419785305695a130234f0fdabc2
Author: Rihyeon Kim <rihyeon8648@gmail.com>

PCI: Free the device name on the pci_setup_device() error path
    
This commit frees the device name in the pci_setup_device() error path
for unknown header types. Since device_initialize() hasn't been called,
pci_scan_device() simply uses kfree() to release the device, which
previously leaked the allocated name.

Link: https://lore.kernel.org/linux-pci/20220817084444.2876059-1-yangyingliang@huawei.com/

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 3e6f0eb7d456c..74d84093943bb 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2192,6 +2192,8 @@ int pci_setup_device(struct pci_dev *dev)
>  	default:				    /* unknown header */
>  		pci_err(dev, "unknown header type %02x, ignoring device\n",
>  			dev->hdr_type);
> +		kfree_const(dev->dev.kobj.name);
> +		dev->dev.kobj.name = NULL;
>  		pci_release_of_node(dev);
 
[Severity: High]
This isn't a bug introduced by this patch, but since we're fixing leaks on
this exact error path, does this also leak memory allocated by early fixups?

Earlier in pci_setup_device(), we execute:

    pci_fixup_device(pci_fixup_early, dev);

Some of these early fixups dynamically allocate memory and attach it to the
device structure. For example, quirk_intel_qat_vf_cap() allocates state
buffers:

drivers/pci/quirks.c:quirk_intel_qat_vf_cap() {
    ...
    state = kzalloc(sizeof(*state) + size, GFP_KERNEL);
    ...
    pci_add_saved_cap(pdev, state);
}

Similarly, pci_add_dma_alias() allocates a bitmap:

drivers/pci/pci.c:pci_add_dma_alias() {
    ...
    dev->dma_alias_mask = bitmap_zalloc(MAX_NR_DEVFNS, GFP_KERNEL);
    ...
}

Because pci_scan_device() handles the -EIO return by directly calling
kfree(dev) instead of going through standard device release, it seems these
dynamically allocated resources (dev->saved_cap_space, dev->dma_alias_mask)
will also be permanently leaked here.

>  		return -EIO;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726093200.146111-1-rihyeon8648@gmail.com?part=2