[PATCH v2 0/3] PCI: rzg3s: Fix issues in cleanup paths

[PATCH v2 0/3] PCI: rzg3s: Fix issues in cleanup paths

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi all,

This series fixes a few issues in the cleanup paths of the Renesas RZ/G3S
PCIe host controller driver. The fixes include:
- Disabling the PCIe reference clock on probe failure.
- Propagating the original error code from platform_get_irq_byname() to
allow probe deferral.
- Unwinding chained handler registration on error in
rzg3s_pcie_init_irqdomain() and clearing chained handlers in
rzg3s_pcie_teardown_irqdomain() to avoid leaving stale handler and data
pointers attached to the parent interrupts.

Note:
- This patch applies on top of pci-next (commit - 90c4d88be15e).
- There is a patch [0] inflight for this driver which doesn't apply
  on pci-next. As there is a conflict between the two, the patch [0]
  will be rebased on top of this series and v3 will be sent out for review.

[0] https://lore.kernel.org/all/20260630141720.3938514-1-john.madieu.xa@bp.renesas.com/

Cheers,
Prabhakar

Lad Prabhakar (3):
  PCI: rzg3s: Disable refclk on probe failure
  PCI: rzg3s: Propagate platform_get_irq_byname() errors
  PCI: rzg3s: Fix IRQ domain initialization error handling

 drivers/pci/controller/pcie-rzg3s-host.c | 50 ++++++++++++++++++------
 1 file changed, 37 insertions(+), 13 deletions(-)


base-commit: 90c4d88be15e11c1e922ef93e611b5e28103e37b
-- 
2.54.0

[PATCH v2 1/3] PCI: rzg3s: Disable refclk on probe failure

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

rzg3s_pcie_host_init_port() enables the PCIe reference clock using
clk_prepare_enable(), but if rzg3s_pcie_probe() subsequently fails, for
example because pci_host_probe() returns an error, the probe error path
only releases the clock handle with clk_put().

Balance clk_prepare_enable() by calling clk_disable_unprepare() in the
probe cleanup path before releasing the clock, ensuring the reference
clock is properly disabled on probe failure.

Fixes: 7ef502fb35b28 ("PCI: Add Renesas RZ/G3S host controller driver")
Cc: stable@vger.kernel.org
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---
v1->v2:
- Added RB tag
---
 drivers/pci/controller/pcie-rzg3s-host.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index a1a78bdb7af1..01488660ab08 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -1897,6 +1897,7 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
 	return 0;
 
 host_probe_teardown:
+	clk_disable_unprepare(host->port.refclk);
 	rzg3s_pcie_teardown_irqdomain(host);
 	host->data->config_deinit(host);
 rpm_put:
-- 
2.54.0

[PATCH v2 2/3] PCI: rzg3s: Propagate platform_get_irq_byname() errors

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

platform_get_irq_byname() may return specific error codes such as
-EPROBE_DEFER, but rzg3s_pcie_init_irqdomain() replaces all failures
with -EINVAL.

This breaks probe deferral by preventing the driver core from retrying
probe once the interrupt provider becomes available.

Propagate the original error code instead.

Fixes: 7ef502fb35b28 ("PCI: Add Renesas RZ/G3S host controller driver")
Cc: stable@vger.kernel.org
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---
v1->v2:
- Added RB tag
---
 drivers/pci/controller/pcie-rzg3s-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index 01488660ab08..f51b5d7c1998 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -1019,7 +1019,7 @@ static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
 
 		irq = platform_get_irq_byname(pdev, irq_name);
 		if (irq < 0)
-			return dev_err_probe(dev, -EINVAL,
+			return dev_err_probe(dev, irq,
 					     "Failed to parse and map INT%c IRQ\n",
 					     'A' + i);
 
-- 
2.54.0

[PATCH v2 3/3] PCI: rzg3s: Fix IRQ domain initialization error handling

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

rzg3s_pcie_init_irqdomain() installs chained handlers for the INTx
parent interrupts before creating the INTx irqdomain and initializing
MSI support.

If any subsequent step fails, such as obtaining an INTx IRQ,
creating the irqdomain, or initializing MSI, the error path returns
without removing any chained handlers that were already installed.
This leaves stale handler and data pointers attached to the parent
interrupts.

Fix the error path by unwinding the chained handler registration before
returning. Also clear the chained handlers during
rzg3s_pcie_teardown_irqdomain() so that teardown fully mirrors the
initialization sequence.

Fixes: 7ef502fb35b28 ("PCI: Add Renesas RZ/G3S host controller driver")
Cc: stable@vger.kernel.org
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- Restricted the code to max 80 columns.
- Moved calling irq_domain_remove() under err_cleanup_intx label.
---
 drivers/pci/controller/pcie-rzg3s-host.c | 49 +++++++++++++++++-------
 1 file changed, 36 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index f51b5d7c1998..70f1faf7e422 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -1006,22 +1006,35 @@ static const struct irq_domain_ops rzg3s_pcie_intx_domain_ops = {
 	.xlate = irq_domain_xlate_onetwocell,
 };
 
+static void rzg3s_pcie_clear_intx_handlers(struct rzg3s_pcie_host *host,
+					   int count)
+{
+	while (--count >= 0) {
+		irq_set_chained_handler_and_data(host->intx_irqs[count], NULL,
+						 NULL);
+	}
+}
+
 static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
 {
 	struct device *dev = host->dev;
 	struct platform_device *pdev = to_platform_device(dev);
+	int i, ret;
 
-	for (int i = 0; i < PCI_NUM_INTX; i++) {
+	for (i = 0; i < PCI_NUM_INTX; i++) {
 		char irq_name[5] = {0};
 		int irq;
 
 		scnprintf(irq_name, ARRAY_SIZE(irq_name), "int%c", 'a' + i);
 
 		irq = platform_get_irq_byname(pdev, irq_name);
-		if (irq < 0)
-			return dev_err_probe(dev, irq,
-					     "Failed to parse and map INT%c IRQ\n",
-					     'A' + i);
+		if (irq < 0) {
+			ret = irq;
+			dev_err_probe(dev, ret,
+				      "Failed to parse and map INT%c IRQ\n",
+				      'A' + i);
+			goto err_cleanup_intx;
+		}
 
 		host->intx_irqs[i] = irq;
 		irq_set_chained_handler_and_data(irq,
@@ -1033,21 +1046,29 @@ static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
 						     PCI_NUM_INTX,
 						     &rzg3s_pcie_intx_domain_ops,
 						     host);
-	if (!host->intx_domain)
-		return dev_err_probe(dev, -EINVAL,
-				     "Failed to add irq domain for INTx IRQs\n");
+	if (!host->intx_domain) {
+		ret = -EINVAL;
+		dev_err_probe(dev, ret,
+			      "Failed to add irq domain for INTx IRQs\n");
+		goto err_cleanup_intx;
+	}
 	irq_domain_update_bus_token(host->intx_domain, DOMAIN_BUS_WIRED);
 
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		int ret = rzg3s_pcie_init_msi(host);
+		ret = rzg3s_pcie_init_msi(host);
 
-		if (ret) {
-			irq_domain_remove(host->intx_domain);
-			return ret;
-		}
+		if (ret)
+			goto err_cleanup_intx;
 	}
 
 	return 0;
+
+err_cleanup_intx:
+	if (host->intx_domain)
+		irq_domain_remove(host->intx_domain);
+	rzg3s_pcie_clear_intx_handlers(host, i);
+
+	return ret;
 }
 
 static void rzg3s_pcie_teardown_irqdomain(struct rzg3s_pcie_host *host)
@@ -1055,6 +1076,8 @@ static void rzg3s_pcie_teardown_irqdomain(struct rzg3s_pcie_host *host)
 	if (IS_ENABLED(CONFIG_PCI_MSI))
 		rzg3s_pcie_teardown_msi(host);
 
+	rzg3s_pcie_clear_intx_handlers(host, PCI_NUM_INTX);
+
 	irq_domain_remove(host->intx_domain);
 }
 
-- 
2.54.0

Re: [PATCH v2 1/3] PCI: rzg3s: Disable refclk on probe failure

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing teardown of chained IRQ handlers for INTx on probe error paths leads to Use-After-Free.
--

--- Patch [1]: [PATCH v2 1/3] PCI: rzg3s: Disable refclk on probe failure ---
commit ff6eba030bfffe106a9c1c6eaf5347fb573ec30c
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

PCI: rzg3s: Disable refclk on probe failure

This commit balances clk_prepare_enable() by calling clk_disable_unprepare()
in the probe cleanup path, ensuring the reference clock is properly disabled
when pci_host_probe() or other subsequent steps fail.

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index a1a78bdb7af11..01488660ab081 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -1897,6 +1897,7 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
>  	return 0;
>  
>  host_probe_teardown:
> +	clk_disable_unprepare(host->port.refclk);
>  	rzg3s_pcie_teardown_irqdomain(host);

[Severity: High]
This isn't a bug introduced by this patch, but I wanted to ask about this
teardown path.

rzg3s_pcie_teardown_irqdomain() removes the IRQ domain but fails to unset the
chained handlers via irq_set_chained_handler_and_data(..., NULL, NULL).

If rzg3s_pcie_init_irqdomain() successfully sets the chained handlers using
the dynamically allocated host structure:

drivers/pci/controller/pcie-rzg3s-host.c:rzg3s_pcie_init_irqdomain() {
        ...
        irq_set_chained_handler_and_data(irq,
                                         rzg3s_pcie_intx_irq_handler,
                                         host);
        ...
}

and a subsequent probe step fails, the devm framework will free the host
structure.

If an INTx platform interrupt triggers afterwards, will the handler
dereference the freed host pointer and cause a use-after-free?

>  	host->data->config_deinit(host);
>  rpm_put:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727095843.116950-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=1

Re: [PATCH v2 2/3] PCI: rzg3s: Propagate platform_get_irq_byname() errors

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing cleanup of chained IRQ handlers on error paths leaves dangling pointers to freed driver memory, leading to a Use-After-Free when probe is deferred.
--

--- Patch [2]: [PATCH v2 2/3] PCI: rzg3s: Propagate platform_get_irq_byname() errors ---
commit 682dcda0e92e75148db8e1626ef00a4a02243870
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

PCI: rzg3s: Propagate platform_get_irq_byname() errors

This commit updates rzg3s_pcie_init_irqdomain() to propagate the original
error code from platform_get_irq_byname() instead of always returning
-EINVAL. This allows the driver core to properly retry probing when the
interrupt provider returns -EPROBE_DEFER.

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 01488660ab081..f51b5d7c19983 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -1019,7 +1019,7 @@ static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
>  
>  		irq = platform_get_irq_byname(pdev, irq_name);
>  		if (irq < 0)
> -			return dev_err_probe(dev, -EINVAL,
> +			return dev_err_probe(dev, irq,
>  					     "Failed to parse and map INT%c IRQ\n",
>  					     'A' + i);

[Severity: High]
This is a pre-existing issue, but does this direct return leave dangling
chained handlers if a subsequent IRQ fails to probe?

Looking at rzg3s_pcie_init_irqdomain(), chained handlers are iteratively
installed on parent interrupts via irq_set_chained_handler_and_data().
If platform_get_irq_byname() fails for a subsequent IRQ (e.g., returning
-EPROBE_DEFER for "intb"), the function returns an error without invoking
irq_set_chained_handler_and_data() with NULL arguments to unregister the
previously installed handlers.

Since this patch explicitly enables probe deferral, this error path becomes
highly reachable. When probe is deferred, devm automatically frees the host
structure, but the IRQ subsystem retains the chained handlers.

If the parent interrupt triggers before the device is reprobed, would
rzg3s_pcie_intx_irq_handler() dereference the freed host pointer and cause
a use-after-free?

It appears this missing cleanup also exists if rzg3s_pcie_init_msi() fails
later in the same function, and during rzg3s_pcie_teardown_irqdomain(),
which removes the domains but leaves the chained handlers registered.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727095843.116950-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=2

Re: [PATCH v2 3/3] PCI: rzg3s: Fix IRQ domain initialization error handling

[sashiko-bot]

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

New issues:
- [High] The err_cleanup_intx error path destroys the INTx IRQ domain before clearing the active chained handlers, creating a use-after-free race.

Pre-existing issues:
- [High] rzg3s_pcie_intx_irq_handler() incorrectly assumes Linux virtual IRQ numbers are contiguous, which can lead to out-of-bounds domain mapping lookups and interrupt storms.
--

--- Patch [3]: [PATCH v2 3/3] PCI: rzg3s: Fix IRQ domain initialization error handling ---
commit 895e108af15957f16a4f68cbf1072bd988ee57dc
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

PCI: rzg3s: Fix IRQ domain initialization error handling

This commit attempts to fix a resource leak by unwinding chained handler
registrations on error paths in rzg3s_pcie_init_irqdomain(). It also adds
teardown logic to clear the handlers, mirroring the initialization sequence.

Cc: stable@vger.kernel.org

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index f51b5d7c19983..70f1faf7e4227 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c

[ ... ]

> @@ -1006,22 +1006,35 @@ static const struct irq_domain_ops rzg3s_pcie_intx_domain_ops = {
[ ... ]
>  static int rzg3s_pcie_init_irqdomain(struct rzg3s_pcie_host *host)
>  {
>  	struct device *dev = host->dev;
>  	struct platform_device *pdev = to_platform_device(dev);
> +	int i, ret;
>  
> -	for (int i = 0; i < PCI_NUM_INTX; i++) {
> +	for (i = 0; i < PCI_NUM_INTX; i++) {
>  		char irq_name[5] = {0};
>  		int irq;
>  
>  		scnprintf(irq_name, ARRAY_SIZE(irq_name), "int%c", 'a' + i);
>  
>  		irq = platform_get_irq_byname(pdev, irq_name);
> -		if (irq < 0)
> -			return dev_err_probe(dev, irq,
> -					     "Failed to parse and map INT%c IRQ\n",
> -					     'A' + i);
> +		if (irq < 0) {
> +			ret = irq;
> +			dev_err_probe(dev, ret,
> +				      "Failed to parse and map INT%c IRQ\n",
> +				      'A' + i);
> +			goto err_cleanup_intx;
> +		}
>  
>  		host->intx_irqs[i] = irq;
>  		irq_set_chained_handler_and_data(irq,
>  						 rzg3s_pcie_intx_irq_handler,
>  						 host);
>  	}

[Severity: High]
This is a pre-existing issue, but does this code assume that the Linux virtual
IRQ numbers returned by platform_get_irq_byname() are always contiguous?

Looking at rzg3s_pcie_intx_irq_handler():

drivers/pci/controller/pcie-rzg3s-host.c:rzg3s_pcie_intx_irq_handler() {
    ...
    unsigned int irq = irq_desc_get_irq(desc);
    u32 intx = irq - host->intx_irqs[0];

    chained_irq_enter(chip, desc);
    generic_handle_domain_irq(host->intx_domain, intx);
    ...
}

If the dynamically allocated virtual IRQs are not strictly sequential, could
this calculation yield an out-of-bounds index for generic_handle_domain_irq()?

If the mapping fails to resolve, could the incoming level-triggered interrupt
remain unacknowledged, leading to an interrupt storm?

[ ... ]

>  	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> -		int ret = rzg3s_pcie_init_msi(host);
> +		ret = rzg3s_pcie_init_msi(host);
>  
> -		if (ret) {
> -			irq_domain_remove(host->intx_domain);
> -			return ret;
> -		}
> +		if (ret)
> +			goto err_cleanup_intx;
>  	}
>  
>  	return 0;
> +
> +err_cleanup_intx:
> +	if (host->intx_domain)
> +		irq_domain_remove(host->intx_domain);
> +	rzg3s_pcie_clear_intx_handlers(host, i);

[Severity: High]
Is there a race condition in this error path?

If MSI initialization fails, irq_domain_remove(host->intx_domain) is called
before rzg3s_pcie_clear_intx_handlers().

Since irq_set_chained_handler_and_data() was already called earlier, the
interrupts are unmasked and active.

If a hardware INTx interrupt fires before the handlers are cleared, could
rzg3s_pcie_intx_irq_handler() run and dereference the freed host->intx_domain
via generic_handle_domain_irq(), causing a use-after-free?

Should rzg3s_pcie_clear_intx_handlers() be called before destroying the IRQ
domain?

> +
> +	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727095843.116950-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=3