[PATCH] clk: tegra: tegra124-emc: Fix memory leak in load_timings_from_dt() on krealloc() failure

[PATCH] clk: tegra: tegra124-emc: Fix memory leak in load_timings_from_dt() on krealloc() failure

[Wentao Liang]

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@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.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;
 	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;
 		return -ENOMEM;
+	}
 
+	tegra->timings = new_timings;
 	timings_ptr = tegra->timings + tegra->num_timings;
 	tegra->num_timings += child_count;
 
-- 
2.34.1

Re: [PATCH] clk: tegra: tegra124-emc: Fix memory leak in load_timings_from_dt() on krealloc() failure

[Markus Elfring]

…
> This fix uses a temporary variable …

Would a corresponding imperative wording become helpful for an improved change description?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc4#n94

Regards,
Markus

Re: [PATCH] clk: tegra: tegra124-emc: Fix memory leak in load_timings_from_dt() on krealloc() failure

[Thierry Reding]

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

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@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.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

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