Linux PCI subsystem development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Vladimir Oltean" <vladimir.oltean@nxp.com>
Cc: linux-pci@vger.kernel.org
Subject: Re: [PATCH v9 1/3] PCI: cadence: Preserve all error codes in cdns_plat_pcie_probe()
Date: Mon, 10 Aug 2026 12:56:41 +0000	[thread overview]
Message-ID: <20260810125641.A875C1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260810124315.433448-2-vladimir.oltean@nxp.com>

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

Pre-existing issues:
- [High] The driver lacks a `.remove` callback and does not set `.suppress_bind_attrs = true`, leading to severe resource leaks and a potential use-after-free when unbound via sysfs.
--

--- Patch [1]: [PATCH v9 1/3] PCI: cadence: Preserve all error codes in cdns_plat_pcie_probe() ---
commit 8180a8544f7ea362a9f7505de96e98a724635cba
Author: Vladimir Oltean <vladimir.oltean@nxp.com>

PCI: cadence: Preserve all error codes in cdns_plat_pcie_probe()

This commit modifies the error path of cdns_plat_pcie_probe() to correctly
propagate error codes instead of returning zero. Errors from pm_runtime_get_sync()
and setup functions are now correctly handled.

> diff --git a/drivers/pci/controller/cadence/pcie-cadence-plat.c b/drivers/pci/controller/cadence/pcie-cadence-plat.c
> index a1ea24fc3b63f..8aed8b4c3e11c 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-plat.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-plat.c
> @@ -126,7 +126,7 @@ static int cdns_plat_pcie_probe(struct platform_device *pdev)
>  	while (phy_count--)
>  		device_link_del(cdns_plat_pcie->pcie->link[phy_count]);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static void cdns_plat_pcie_shutdown(struct platform_device *pdev)

[Severity: High]
This is a pre-existing issue, but since you are fixing the probe path, could we
also address the missing driver removal handling?

Looking at cdns_plat_pcie_driver:

static struct platform_driver cdns_plat_pcie_driver = {
	.driver = {
		.name = "cdns-pcie",
		.of_match_table = cdns_plat_pcie_of_match,
		.pm = &cdns_pcie_pm_ops,
	},
	.probe = cdns_plat_pcie_probe,
	.shutdown = cdns_plat_pcie_shutdown,
};

The driver lacks a .remove callback and does not set .suppress_bind_attrs = true.

If a user unbinds the driver via sysfs or unloads the module, the platform
driver core will successfully unbind it. The devm subsystem will then
automatically free all managed resources, including cdns_plat_pcie, the host
bridge, and memory mappings.

However, since pci_stop_root_bus() and pci_remove_root_bus() are never explicitly
called, the PCI root bus and its child devices remain registered in the system.

Could any subsequent access to the PCI bus (such as running lspci) dereference
the freed host bridge structure and cause a use-after-free panic?

Would it make sense to set .suppress_bind_attrs = true to prevent unbinding
via sysfs, or implement a proper .remove callback?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810124315.433448-1-vladimir.oltean@nxp.com?part=1

  reply	other threads:[~2026-08-10 12:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-10 12:43 [PATCH v9 0/3] PCI prerequisites for PHY consumer/provider split Vladimir Oltean
2026-08-10 12:43 ` [PATCH v9 1/3] PCI: cadence: Preserve all error codes in cdns_plat_pcie_probe() Vladimir Oltean
2026-08-10 12:56   ` sashiko-bot [this message]
2026-08-10 12:43 ` [PATCH v9 2/3] PCI: Add missing headers transitively included by <linux/phy/phy.h> Vladimir Oltean
2026-08-10 12:46   ` sashiko-bot
2026-08-10 12:43 ` [PATCH v9 3/3] PCI: Remove device links to PHY Vladimir Oltean
2026-08-10 12:58   ` sashiko-bot

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=20260810125641.A875C1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vladimir.oltean@nxp.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox