drivers/clk/tegra/clk-tegra124-emc.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
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
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>
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
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>
© 2016 - 2026 Red Hat, Inc.