Received-SPF: pass (google.com: domain of linux-kernel+bounces-29844-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <efc748d7-1a67-476a-82a4-206a9ed252cb@web.de>
Date: Thu, 18 Jan 2024 09:06:50 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] drm/msm/adreno: Add a null pointer check in
 zap_shader_load_mdt()
To: Kunwu Chan <chentao@kylinos.cn>, freedreno@lists.freedesktop.org,
 dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org,
 kernel-janitors@vger.kernel.org, Abhinav Kumar <quic_abhinavk@quicinc.com>,
 Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
 Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@gmail.com>,
 Marijn Suijten <marijn.suijten@somainline.org>,
 Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>
Cc: LKML <linux-kernel@vger.kernel.org>
References: <20240116032732.65262-1-chentao@kylinos.cn>
 <9e390783-05c5-47fc-a0c6-b95e249fe691@web.de>
 <4d4f4ae4-e5e9-4d9e-ac25-d262e7ea23fc@kylinos.cn>
Content-Language: en-GB
From: Markus Elfring <Markus.Elfring@web.de>
In-Reply-To: <4d4f4ae4-e5e9-4d9e-ac25-d262e7ea23fc@kylinos.cn>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:18ZZnxsG3g8=;b24lAmbYFzSGdfKDOiYpU/Be2jm
 QMSpsqM4qlNwDmDXCamIfP5CrdkxIwQ9i0lr34c0ts6NUR273+ajaQ6psrjP84YyImtl6ZsJV
 0CDVAarmiQfPQUcD8PqSF6AfSZ8ZZ5vvSs3X0WwgZV6XHYT7bBXzxapU4rw38Hlyxyz2Xu51v
 rQd3Uevo1E3t/9GYTRLh0GKK+Y6egUCILRHenil2fqYVDCau6FerYRu3Xl9UK/iMnaI6QUNa7
 78LNOloCvwz2om2K1IOMQhhmjVQF/6CO3Doljyt34R4Q36aUknoxQos6h9MNtji3rbiPlrU0I
 URqLa/AhzBElY8yskiPXD1oe6el+PptLeNGrFhy5gAQTCq8WDR2sztea620dlWDSrXR/JSpnp
 7m/1cP7VsstjmMA+9AgVSnE5+qxvbEIhJr9qi6fGUq5ZJzAj0igir8+magl2aA1CBEZzvoLzZ
 /O8NH2WjmaJdXCdOy5NTkm5QZgQIKRekcZvMUt/dxXUGtHz8EXP5s3CQkqzx9Rk3hi9F11nHJ
 XesEvME19ykj/83a/7rc6gvWoxngWfehQVzBF1ZJ6Q0HX6pQBJ5c1jscLdDFnwpE6jEVklS/v
 tPRdt3MiSWMbfxxHXzNi14/nhPtZDo05rOP5mS6Qzx46MhW8oCS9fFLCPXO5eTx/AL16Y/9yW
 tJEFAAOWbUbGaTIu2x4QWB0IBdQj023UhjVb46fwjhQB5hC0pdzTw2tfUY6qM74e/9JPqniQ9
 GxJMWvi61sJejsuxcK7b8qXR0CiPr9oLv2NarKmudEAkFTKTPegiDtVlgpZU81E0mYhZAh3kC
 uM9TGebh1Dccv9QMHzijeANwnGeyc1L5ss2Cg/YaJG18XQeHclIj9Ipe0jJ01p0jhSrHLWsL0
 PuF2aPJ1Qo+j+rnv6EiaFyi7zSSckePNnmvbGWA7XtDV0TQd/wQnWV6s3CJ8Y7iiOps84DmPG
 blH2lA==

>>> kasprintf() returns a pointer to dynamically allocated memory
>>> which can be NULL upon failure. Ensure the allocation was successful
>>> by checking the pointer validity.
>> =E2=80=A6
>>> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>>> @@ -144,6 +144,10 @@ static int zap_shader_load_mdt(struct msm_gpu *gp=
u, const char *fwname,
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 char *newname;
>>>
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 newname =3D kas=
printf(GFP_KERNEL, "qcom/%s", fwname);
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!newname) {
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 re=
t =3D -ENOMEM;
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 go=
to out;
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>> =E2=80=A6
>>
>> How do you think about to avoid the repetition of the pointer check
>> for the variable =E2=80=9Cmem_region=E2=80=9D?
> "mem_region"? Is this a clerical error, do you mean 'newname'?

Please take another look at implementation details:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/dr=
ivers/gpu/drm/msm/adreno/adreno_gpu.c?h=3Dv6.7#n124


=E2=80=A6
>> Can the usage of other labels become more appropriate?

I propose to reconsider also the influence of the label =E2=80=9Cout=E2=80=
=9D here.
https://elixir.bootlin.com/linux/v6.7/source/drivers/gpu/drm/msm/adreno/ad=
reno_gpu.c#L167

Regards,
Markus