[PATCH] PCI: rcar-gen4: Fix device_node leak in rcar_gen4_pcie_host_msi_addr()

Fuad Tabba posted 1 patch 6 days, 14 hours ago
There is a newer version of this series
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] PCI: rcar-gen4: Fix device_node leak in rcar_gen4_pcie_host_msi_addr()
Posted by Fuad Tabba 6 days, 14 hours ago
rcar_gen4_pcie_host_msi_addr() calls of_msi_xlate() with *msi_np NULL,
so it receives the MSI controller node with a reference held, and every
return past the NULL check leaks that reference, the success path
included. Declare msi_node with __free(device_node) so it's put on
every return.

Fixes: 8d6af27c0a73 ("PCI: rcar-gen4: Configure AXIINTC if iMSI-RX is not used")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-pci/20260905213855.8D6671F00A3D@smtp.kernel.org/
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index fbe465a29068f..d61ce802b4614 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -323,7 +323,7 @@ static struct rcar_gen4_pcie *rcar_gen4_pcie_alloc(struct platform_device *pdev)
 static int rcar_gen4_pcie_host_msi_addr(struct dw_pcie_rp *pp, u32 *msi_addr)
 {
 	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
-	struct device_node *msi_node = NULL;
+	struct device_node *msi_node __free(device_node) = NULL;
 	struct device *dev = dw->dev;
 	struct resource res;
 	u64 addr;

base-commit: fd73f4a6659897191fa0d40695fe370925dd3780
-- 
2.39.5
Re: [PATCH] PCI: rcar-gen4: Fix device_node leak in rcar_gen4_pcie_host_msi_addr()
Posted by Marek Vasut 3 days, 2 hours ago
On 9/18/26 11:10 AM, Fuad Tabba wrote:
> rcar_gen4_pcie_host_msi_addr() calls of_msi_xlate() with *msi_np NULL,
> so it receives the MSI controller node with a reference held, and every
> return past the NULL check leaks that reference, the success path
> included. Declare msi_node with __free(device_node) so it's put on
> every return.
> 
> Fixes: 8d6af27c0a73 ("PCI: rcar-gen4: Configure AXIINTC if iMSI-RX is not used")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/linux-pci/20260905213855.8D6671F00A3D@smtp.kernel.org/
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---
>   drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index fbe465a29068f..d61ce802b4614 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -323,7 +323,7 @@ static struct rcar_gen4_pcie *rcar_gen4_pcie_alloc(struct platform_device *pdev)
>   static int rcar_gen4_pcie_host_msi_addr(struct dw_pcie_rp *pp, u32 *msi_addr)
>   {
>   	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> -	struct device_node *msi_node = NULL;
> +	struct device_node *msi_node __free(device_node) = NULL;
I think you have to call of_node_put() on msi_node(), so what about this 
instead ?

"
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c 
b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 8057c31c0123a..2eb20cff2fcad 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -382,20 +382,29 @@ static int rcar_gen4_pcie_host_msi_addr(struct 
dw_pcie_rp *pp, u32 *msi_addr)
  		return -ENODEV;

  	/* Check if "msi-parent" or the "msi-map" points to ARM GICv3 ITS. */
-	if (!of_device_is_compatible(msi_node, "arm,gic-v3-its"))
-		return dev_err_probe(dev, -ENODEV, "Compatible MSI controller not 
found\n");
+	if (!of_device_is_compatible(msi_node, "arm,gic-v3-its")) {
+		ret = dev_err_probe(dev, -ENODEV, "Compatible MSI controller not 
found\n");
+		goto exit;
+	}

  	/* Derive GITS_TRANSLATER address from GICv3 */
  	ret = of_address_to_resource(msi_node, 0, &res);
-	if (ret < 0)
-		return dev_err_probe(dev, ret, "MSI controller resources not 
obtained\n");
+	if (ret < 0) {
+		ret = dev_err_probe(dev, ret, "MSI controller resources not obtained\n");
+		goto exit;
+	}

  	addr = res.start + GITS_TRANSLATER;
-	if (addr >= SZ_4G)
-		return dev_err_probe(dev, -EINVAL, "MSI controller address above 
32bit range\n");
+	if (addr >= SZ_4G) {
+		ret = dev_err_probe(dev, -EINVAL, "MSI controller address above 32bit 
range\n");
+		goto exit;
+	}

  	*msi_addr = addr;
-	return 0;
+
+exit:
+	of_node_put(msi_node);
+	return ret;
  }

  static int rcar_gen4_pcie_host_msi_init(struct dw_pcie_rp *pp)
"

Also, I think drivers/pci/controller/pcie-iproc.c 
iproc_pcie_msi_enable() needs similar fix ?
Re: [PATCH] PCI: rcar-gen4: Fix device_node leak in rcar_gen4_pcie_host_msi_addr()
Posted by Fuad Tabba 2 days, 16 hours ago
Hi Marek,

On Mon, 21 Sep 2026 22:47:30 +0200, Marek Vasut <marek.vasut@mailbox.org> wrote:

[...]

> I think you have to call of_node_put() on msi_node(), so what about this 
> instead ?

The __free(device_node) is that call: it puts msi_node on every return from the function, the success path included, so no goto is needed. It's the same scoped cleanup pci-imx6.c and pcie-rzg3s-host.c use for their device_node lookups.

> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c 

[...]

> "
> 
> Also, I think drivers/pci/controller/pcie-iproc.c 
> iproc_pcie_msi_enable() needs similar fix ?

I don't think so: iproc_pcie_msi_enable() already puts the node at out_put_node, on the steer failure and the success paths alike.

Cheers,
/fuad
Re: [PATCH] PCI: rcar-gen4: Fix device_node leak in rcar_gen4_pcie_host_msi_addr()
Posted by Marek Vasut 2 days, 5 hours ago
On 9/22/26 8:38 AM, Fuad Tabba wrote:

Hello Fuad,

> On Mon, 21 Sep 2026 22:47:30 +0200, Marek Vasut <marek.vasut@mailbox.org> wrote:
> 
> [...]
> 
>> I think you have to call of_node_put() on msi_node(), so what about this
>> instead ?
> 
> The __free(device_node) is that call: it puts msi_node on every return from the function, the success path included, so no goto is needed. It's the same scoped cleanup pci-imx6.c and pcie-rzg3s-host.c use for their device_node lookups.

Oh, this is very nice, TIL, thank you.

Reviewed-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Tested-by: Marek Vasut <marek.vasut+renesas@mailbox.org> # Retronix 
R-Car V4H Sparrow Hawk

>> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> 
> [...]
> 
>> "
>>
>> Also, I think drivers/pci/controller/pcie-iproc.c
>> iproc_pcie_msi_enable() needs similar fix ?
> 
> I don't think so: iproc_pcie_msi_enable() already puts the node at out_put_node, on the steer failure and the success paths alike.

Could you maybe send similar simplification patch ?

Thank you !

-- 
Best regards,
Marek Vasut
Re: [PATCH] PCI: rcar-gen4: Fix device_node leak in rcar_gen4_pcie_host_msi_addr()
Posted by Fuad Tabba 2 days, 2 hours ago
Hi Marek,

On Tue, 22 Sept 2026 at 19:07, Marek Vasut <marek.vasut@mailbox.org> wrote:
[...]
> Oh, this is very nice, TIL, thank you.
>
> Reviewed-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> Tested-by: Marek Vasut <marek.vasut+renesas@mailbox.org> # Retronix
> R-Car V4H Sparrow Hawk

Thanks.

>
> >> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> >
> > [...]
> >
> >> "
> >>
> >> Also, I think drivers/pci/controller/pcie-iproc.c
> >> iproc_pcie_msi_enable() needs similar fix ?
> >
> > I don't think so: iproc_pcie_msi_enable() already puts the node at out_put_node, on the steer failure and the success paths alike.
>
> Could you maybe send similar simplification patch ?

Sure: https://lore.kernel.org/all/20260922210839.3765444-1-fuad.tabba@linux.dev/

Cheers,
/fuad

>
> Thank you !
>
> --
> Best regards,
> Marek Vasut