Received-SPF: pass (google.com: domain of linux-kernel+bounces-30939-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <a5f974aa-826d-421b-bfe2-bb6c9b43ea9a@web.de>
Date: Fri, 19 Jan 2024 10:06:02 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Kunwu Chan <chentao@kylinos.cn>, linux-amlogic@lists.infradead.org,
 linux-arm-kernel@lists.infradead.org, kernel-janitors@vger.kernel.org,
 Jerome Brunet <jbrunet@baylibre.com>, Kevin Hilman <khilman@baylibre.com>,
 Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
 Neil Armstrong <neil.armstrong@linaro.org>
Cc: LKML <linux-kernel@vger.kernel.org>
References: <20240119074635.265218-1-chentao@kylinos.cn>
Subject: Re: [PATCH] meson-mx-socinfo: Fix possible null-pointer dereference
 issues in meson_mx_socinfo_init
Content-Language: en-GB
From: Markus Elfring <Markus.Elfring@web.de>
In-Reply-To: <20240119074635.265218-1-chentao@kylinos.cn>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:UigNfyhaB4k=;UdHpO01shOQaaFwGIPHVxcK+HUZ
 VbqH4e0fYiE5esvL3Xw20BGCS6Cwt3LUi/oLUtC+x9bio1PMIX8BV2MAmQp3BTgYMMF20OV2s
 jqsL5x7oZ0o+ELhtDwM6es78LAWh771mbjZoRvBnaJC+CID8i5WAnCL/4zDH7p0AYLpeLy5Gv
 FB/3CTSv6IRRYiRbs2ORvYZwPgJuFdlyZX8VIcQKygJFT1AadFSzitsqNz5DnRkihyypVIdUs
 rDDY7k8bz1SJu7/2WKLHcuqF30COXLYj4n8Sb1B5Bh6i3X5VWChXMB9riXQn0qix6+P5f6WYb
 Xz/sBicYnxTLFEdn+zL9OVn7nmhKEm8ea2b5Z/fFFHBMY/uTdKL4ZA62LgKhxOEemfQk28Nzq
 2Uhvl5cgCcMptCu7Je+FnAO9YhIU3iENkaphqjl0OF9zi5WnGnMt7/B/Dob4zlkidM3VX9VN8
 oNNOsCvDX/0q1srFjbJw9np1RCttc88P3DJl8sjhLYHl86id10e8U6JC68qkQ59aDvXguINBP
 EbauE4ekg6nsw57Vt4x22stfFyabslQHHx8ORWKZJgk30PB5iU0dJK56ssbDHop23VJY+WAbt
 iQvciVgpuNf0j+CVPcUPS60b2221yJJe2ltNkZS4kOLCtRfP2dFbsDG3Huoxo1vRnONbzLglq
 rso0SDsasPYbKcg1L8UUetmBcoVOT80Z7JWZGPlPXssPTjcqXvCNU5iv5PySJQwYz8hn1aySw
 xxrumNfL/u3EWOwNKRPpfr1aUqNwfpTZ2wJMuUbXHu3DR3DIwtrjXoeqx+1DaNcPD2q0t7a3S
 eppSn08LWTLSaqwodjD3PmTVi8pTRRlbdcEzoFsw6AOVha/U2VNPD7GKCBVftw79J1nwyKkwo
 UuoIn72v07Lx5z6Q+N3yo71kFM48JGGsEqVwUYE3vhSnFS27hm3KCGmJFjtpJ9wrtKOHvEcDU
 abogjg==

> In meson_mx_socinfo_revision, kasprintf() returns a pointer
> to dynamically allocated memory which can be NULL upon failure.
>
> Similarly, the kstrdup_const in the meson_mx_socinfo_soc_id
> returns a null pointer when it fails. Ensure the allocation was successf=
ul
> by checking the pointer validity. Avoid null pointer dereference issues.

Would you like to add the tag =E2=80=9CFixes=E2=80=9D for the completion o=
f the error handling
in this function implementation?


=E2=80=A6
> +++ b/drivers/soc/amlogic/meson-mx-socinfo.c
> @@ -160,6 +160,12 @@ static int __init meson_mx_socinfo_init(void)
>  							   metal_rev);
>  	soc_dev_attr->soc_id =3D meson_mx_socinfo_soc_id(major_ver, metal_rev)=
;
>
> +	if (!soc_dev_attr->revision || !soc_dev_attr->soc_id) {

I suggest to split such a check for null pointers.


> +		kfree_const(soc_dev_attr->revision);
> +		kfree_const(soc_dev_attr->soc_id);
> +		kfree(soc_dev_attr);
> +		return -ENOMEM;
> +	}
>  	soc_dev =3D soc_device_register(soc_dev_attr);
>  	if (IS_ERR(soc_dev)) {
>  		kfree_const(soc_dev_attr->revision);

Please use a goto chain for better exception handling.
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+go=
to+chain+when+leaving+a+function+on+error+when+using+and+releasing+resourc=
es

Regards,
Markus