linux-kernel - Re: [PATCH] irqchip/ls-extirq: fix resource leak in ls_extirq_parse

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87bjpdtf0w.ffs@tglx>
Date: Mon, 21 Jul 2025 23:22:55 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: jackysliu <1972843537@...com>
Cc: linux-kernel@...r.kernel.org, Siyang Liu <1972843537@...com>
Subject: Re: [PATCH] irqchip/ls-extirq: fix resource leak in
 ls_extirq_parse_map()

On Mon, Jul 21 2025 at 12:43, jackysliu wrote:

> From: Siyang Liu <1972843537@...com>
>
> The ls_extirq_parse_map function uses of_find_node_by_phandle() to obtain

ls_extirq_parse_map() uses ...

> interrupt parent nodes within a loop, but fails to release the reference
> count on these nodes. This results in a kernel memory leak as the node
> reference counts continuously increase.

There is no continous increase as each ipar node is only looked up and
referenced exactly once. So this leaks exactly ONE reference count per
node.

> Add of_node_put() calls to properly release the references when the nodes

I see only one call, not several calls...

> are no longer needed
>
> Signed-off-by: Siyang Liu <1972843537@...com>
> ---
>  drivers/irqchip/irq-ls-extirq.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/irqchip/irq-ls-extirq.c b/drivers/irqchip/irq-ls-extirq.c
> index 50a7b38381b9..fb4e7b3a6e2f 100644
> --- a/drivers/irqchip/irq-ls-extirq.c
> +++ b/drivers/irqchip/irq-ls-extirq.c
> @@ -164,6 +164,7 @@ ls_extirq_parse_map(struct ls_extirq_data *priv, struct device_node *node)
>  		for (j = 0; j < intsize; ++j)
>  			priv->map[hwirq].param[j] = be32_to_cpup(map++);
>  		mapsize -= intsize;
> +		of_node_put(ipar);

That's the wrong point as this might not be reached because of the
following checks before the inner loop:

		ret = of_property_read_u32(ipar, "#interrupt-cells", &intsize);
		if (ret)
			return ret;

		if (intsize > mapsize)
			return -EINVAL;

Please don't add put() calls in every error return path. _ONE_ put() at
the proper place is sufficient.

Thanks,

        tglx