linux-kernel - Re: [PATCH] clk: tegra: tegra124-emc: Fix memory leak in load_timings_from

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 for Android: free password hash cracker in your pocket

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

Message-ID: <3wvnt4gywyffvqnxagp6c6h6oxxusboc5bascgng34mjshdamj@a75sqvemckg5>
Date: Fri, 14 Nov 2025 14:30:31 +0100
From: Thierry Reding <thierry.reding@...il.com>
To: Wentao Liang <vulab@...as.ac.cn>
Cc: pdeschrijver@...dia.com, pgaikwad@...dia.com, mturquette@...libre.com, 
	sboyd@...nel.org, jonathanh@...dia.com, linux-clk@...r.kernel.org, 
	linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] clk: tegra: tegra124-emc: Fix memory leak in
 load_timings_from_dt() on krealloc() failure

On Tue, Nov 04, 2025 at 07:42:29AM +0000, Wentao Liang wrote:
> The function load_timings_from_dt() directly assigns the result of
> krealloc() to tegra->timings, which causes a memory leak when
> krealloc() fails. When krealloc() returns NULL, the original pointer
> is lost, making it impossible to free the previously allocated memory.
> 
> This fix uses a temporary variable to store the krealloc() result and
> only updates tegra->timings after successful allocation, preserving
> the original pointer in case of failure.
> 
> Fixes: 888ca40e2843 ("clk: tegra: emc: Support multiple RAM codes")
> Cc: stable@...r.kernel.org
> Signed-off-by: Wentao Liang <vulab@...as.ac.cn>
> ---
>  drivers/clk/tegra/clk-tegra124-emc.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra124-emc.c b/drivers/clk/tegra/clk-tegra124-emc.c
> index 2a6db0434281..ed4972fa6dab 100644
> --- a/drivers/clk/tegra/clk-tegra124-emc.c
> +++ b/drivers/clk/tegra/clk-tegra124-emc.c
> @@ -444,6 +444,7 @@ static int load_timings_from_dt(struct tegra_clk_emc *tegra,
>  				u32 ram_code)
>  {
>  	struct emc_timing *timings_ptr;
> +	struct emc_timing *new_timings;

Can we not reuse timings_ptr to avoid this extra variable?

>  	struct device_node *child;
>  	int child_count = of_get_child_count(node);
>  	int i = 0, err;
> @@ -451,10 +452,15 @@ static int load_timings_from_dt(struct tegra_clk_emc *tegra,
>  
>  	size = (tegra->num_timings + child_count) * sizeof(struct emc_timing);
>  
> -	tegra->timings = krealloc(tegra->timings, size, GFP_KERNEL);
> -	if (!tegra->timings)
> +	new_timings  = krealloc(tegra->timings, size, GFP_KERNEL);
> +	if (!new_timings) {
> +		kfree(tegra->timings);
> +		tegra->timings = NULL;
> +		tegra->num_timings = 0;

These two lines are somewhat pointless. The memory that tegra points to
will be kfree()'ed after we return below and the function will fail, so
nothing will ever access the memory anyway.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)