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

--ktsbmyqbhvpuozjb
Content-Type: text/plain; protected-headers=v1; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PATCH v2] clk: tegra: tegra124-emc: fix krealloc() memory leak
MIME-Version: 1.0

On Sun, May 31, 2026 at 09:52:52PM +0200, Alexander A. Klimov wrote:
> Don't just overwrite the original pointer passed to krealloc()
> with its return value without checking latter:
>=20
>     MEM =3D krealloc(MEM, SZ, GFP);
>=20
> 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:
>=20
>     TMP =3D krealloc(MEM, SZ, GFP);
>     if (!TMP) return;
>     MEM =3D TMP;
>=20
> Fixes: 888ca40e2843 ("clk: tegra: emc: Support multiple RAM codes")
> Signed-off-by: Alexander A. Klimov <[email protected]>
> ---
>  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)
>=20
>  [=E2=9C=93] scripts/checkpatch.pl --strict
>  [=E2=9C=93] allmodconfig compiled (i686, LLVM)
>  [=E2=9C=93] localyesconfig booted (IBM T43)

The latter two are a bit pointless because they are not going to hit
this because the driver depends on ARCH_TEGRA which doesn't exist on
i686 allmodconfig and booting this on a T43 isn't going to hit this
either.

>  Note to myself: use --in-reply-to=3DahilgKKwkttOd9H6@orome

It's not generally useful to send as replies to earlier versions. Just
regular sending is good enough. We have tools to track versions and
such.

>=20
>  drivers/clk/tegra/clk-tegra124-emc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>=20
> 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_em=
c *tegra,
>  {
>  	struct emc_timing *timings_ptr;
>  	int child_count =3D of_get_child_count(node);
> +	struct emc_timing *timings;

I guess we could've reused timings_ptr. Or remove timings_ptr and use
the new one, but I guess this is fine, too:

Reviewed-by: Thierry Reding <[email protected]>

>  	int i =3D 0, err;
>  	size_t size;
> =20
>  	size =3D (tegra->num_timings + child_count) * sizeof(struct emc_timing);
> =20
> -	tegra->timings =3D krealloc(tegra->timings, size, GFP_KERNEL);
> -	if (!tegra->timings)
> +	timings =3D krealloc(tegra->timings, size, GFP_KERNEL);
> +	if (!timings)
>  		return -ENOMEM;
> =20
> +	tegra->timings =3D timings;
>  	timings_ptr =3D tegra->timings + tegra->num_timings;
>  	tegra->num_timings +=3D child_count;

--ktsbmyqbhvpuozjb
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmprc9MACgkQ3SOs138+
s6EAjg/+NggFhcQJXqx0VYk7iklKkEhSAXctBdS27tzGF9iOPvrlVFB0VID0HbBc
aS/b31peBKERQiO/5+gV5/VSxdmYfR8OvD3J/n8LTHCPUJYE/N/Pg3cvlEsAQxTj
sx6X5BO26AXXKzPYQdstF9aqoG1mZIncW16Mw88dRZjDRvyEidz0VRRhKOqSVQam
bPh7hj93Xx8Nys/09XtvvHV0nFaKkxW1b2ONCkZn2Fq2VJQIu8T0gKZ73I4MrE+Z
7fMFa6vt6pc2Pg8cXuFu95Uk1No6yR/qUJChbdWWwjxU5g4kEDrnCc6RMNSM7u8C
Zz9SUi1Qm9llLSVfmKUv8zTutt80txbTSIZ1cDXRDqF1z7rbRSLcB/XVzTqSVNJc
h113U3UfusavpN9Ro2QPhK1Lls4v9MU96N0AP14jT7gH2cqSui/d+8y7FosVMgX6
0KEF7u/aVQSYf/+W3KZSlevQxYQwIX3sgrJWkRSBvT9EtpxqAUItzUESil7kQhY2
MQGsPwGEUzi8s7Vhc/xBOv1wQ84zG0r0Q2ZtT1kcXV+NuiN9sIYINwvsBObE5JUK
pExg11rE3xlIliidR2RugA8P22pqTdBaG321Tl3fl7yl7y83mfSkYbZW0VjP2WiU
ILp7W2SVEoG3yCFKnSD7dj4xTZ/quzUV1na9Ej6F43NUWJZJKNM=
=OIU3
-----END PGP SIGNATURE-----

--ktsbmyqbhvpuozjb--