[PATCH] clk: tegra: tegra124-emc: fix krealloc() memory leak

[PATCH] clk: tegra: tegra124-emc: fix krealloc() memory leak

[Alexander A. Klimov]

Don't just overwrite the original pointer passed to krealloc()
with its return value without checking latter:

    MEM = krealloc(MEM, SZ, GFP);

If krealloc() returns NULL, that erases the pointer
to the still allocated memory, hence leaks this memory.
Instead, use a temporary variable, check it's not NULL
and only then assign it to the original pointer:

    TMP = krealloc(MEM, SZ, GFP);
    if (!TMP) return;
    MEM = TMP;

Fixes: 888ca40e2843 ("clk: tegra: emc: Support multiple RAM codes")
Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
---
 drivers/clk/tegra/clk-tegra124-emc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra124-emc.c b/drivers/clk/tegra/clk-tegra124-emc.c
index f3b2c96fdcfc..8053fbbb06c8 100644
--- a/drivers/clk/tegra/clk-tegra124-emc.c
+++ b/drivers/clk/tegra/clk-tegra124-emc.c
@@ -446,14 +446,13 @@ static int load_timings_from_dt(struct tegra_clk_emc *tegra,
 	struct emc_timing *timings_ptr;
 	int child_count = of_get_child_count(node);
 	int i = 0, err;
-	size_t size;
+	size_t size = (tegra->num_timings + child_count) * sizeof(struct emc_timing);
+	void *mem = krealloc(tegra->timings, size, GFP_KERNEL);
 
-	size = (tegra->num_timings + child_count) * sizeof(struct emc_timing);
-
-	tegra->timings = krealloc(tegra->timings, size, GFP_KERNEL);
-	if (!tegra->timings)
+	if (!mem)
 		return -ENOMEM;
 
+	tegra->timings = mem;
 	timings_ptr = tegra->timings + tegra->num_timings;
 	tegra->num_timings += child_count;
 
-- 
2.54.0

Re: [PATCH] clk: tegra: tegra124-emc: fix krealloc() memory leak

[Brian Masney]

On Tue, May 26, 2026 at 08:13:13AM +0200, Alexander A. Klimov wrote:
> Don't just overwrite the original pointer passed to krealloc()
> with its return value without checking latter:
> 
>     MEM = krealloc(MEM, SZ, GFP);
> 
> If krealloc() returns NULL, that erases the pointer
> to the still allocated memory, hence leaks this memory.
> Instead, use a temporary variable, check it's not NULL
> and only then assign it to the original pointer:
> 
>     TMP = krealloc(MEM, SZ, GFP);
>     if (!TMP) return;
>     MEM = TMP;
> 
> Fixes: 888ca40e2843 ("clk: tegra: emc: Support multiple RAM codes")
> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>

Reviewed-by: Brian Masney <bmasney@redhat.com>

Re: [PATCH] clk: tegra: tegra124-emc: fix krealloc() memory leak

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1698 bytes --]

On Tue, May 26, 2026 at 08:13:13AM +0200, Alexander A. Klimov wrote:
> Don't just overwrite the original pointer passed to krealloc()
> with its return value without checking latter:
> 
>     MEM = krealloc(MEM, SZ, GFP);
> 
> If krealloc() returns NULL, that erases the pointer
> to the still allocated memory, hence leaks this memory.
> Instead, use a temporary variable, check it's not NULL
> and only then assign it to the original pointer:
> 
>     TMP = krealloc(MEM, SZ, GFP);
>     if (!TMP) return;
>     MEM = TMP;
> 
> Fixes: 888ca40e2843 ("clk: tegra: emc: Support multiple RAM codes")
> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
> ---
>  drivers/clk/tegra/clk-tegra124-emc.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra124-emc.c b/drivers/clk/tegra/clk-tegra124-emc.c
> index f3b2c96fdcfc..8053fbbb06c8 100644
> --- a/drivers/clk/tegra/clk-tegra124-emc.c
> +++ b/drivers/clk/tegra/clk-tegra124-emc.c
> @@ -446,14 +446,13 @@ static int load_timings_from_dt(struct tegra_clk_emc *tegra,
>  	struct emc_timing *timings_ptr;
>  	int child_count = of_get_child_count(node);
>  	int i = 0, err;
> -	size_t size;
> +	size_t size = (tegra->num_timings + child_count) * sizeof(struct emc_timing);
> +	void *mem = krealloc(tegra->timings, size, GFP_KERNEL);
>  
> -	size = (tegra->num_timings + child_count) * sizeof(struct emc_timing);

This looks really wild now. I think it'd be better to follow the
original style:

	size_t size;
	void *mem;

	size = ...;

	mem = krealloc(...);
	if (!mem)
		...

With that:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2] clk: tegra: tegra124-emc: fix krealloc() memory leak

[Alexander A. Klimov]

Don't just overwrite the original pointer passed to krealloc()
with its return value without checking latter:

    MEM = krealloc(MEM, SZ, GFP);

If krealloc() returns NULL, that erases the pointer
to the still allocated memory, hence leaks this memory.
Instead, use a temporary variable, check it's not NULL
and only then assign it to the original pointer:

    TMP = krealloc(MEM, SZ, GFP);
    if (!TMP) return;
    MEM = TMP;

Fixes: 888ca40e2843 ("clk: tegra: emc: Support multiple RAM codes")
Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
---
 v2: Separate variable declaration/init
 v2: While on it, enhance variable name
 v2: While on it, explicit variable type
 v2: While on it, re-order variables (reverse Xmas tree)

 [✓] scripts/checkpatch.pl --strict
 [✓] allmodconfig compiled (i686, LLVM)
 [✓] localyesconfig booted (IBM T43)

 Note to myself: use --in-reply-to=ahilgKKwkttOd9H6@orome

 drivers/clk/tegra/clk-tegra124-emc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra124-emc.c b/drivers/clk/tegra/clk-tegra124-emc.c
index f3b2c96fdcfc..81e4c02a807c 100644
--- a/drivers/clk/tegra/clk-tegra124-emc.c
+++ b/drivers/clk/tegra/clk-tegra124-emc.c
@@ -445,15 +445,17 @@ static int load_timings_from_dt(struct tegra_clk_emc *tegra,
 {
 	struct emc_timing *timings_ptr;
 	int child_count = of_get_child_count(node);
+	struct emc_timing *timings;
 	int i = 0, err;
 	size_t size;
 
 	size = (tegra->num_timings + child_count) * sizeof(struct emc_timing);
 
-	tegra->timings = krealloc(tegra->timings, size, GFP_KERNEL);
-	if (!tegra->timings)
+	timings = krealloc(tegra->timings, size, GFP_KERNEL);
+	if (!timings)
 		return -ENOMEM;
 
+	tegra->timings = timings;
 	timings_ptr = tegra->timings + tegra->num_timings;
 	tegra->num_timings += child_count;
 
-- 
2.54.0