[PATCH] drm/nouveau/gsp: Fix possible NULL pointer dereference warning in r535_dmac_alloc

sunliming@linux.dev posted 1 patch 1 month, 4 weeks ago
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] drm/nouveau/gsp: Fix possible NULL pointer dereference warning in r535_dmac_alloc
Posted by sunliming@linux.dev 1 month, 4 weeks ago
From: sunliming <sunliming@kylinos.cn>

Fix below smatch warnings:
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c:178 r535_dmac_alloc()
warn: 'args' can also be NULL

Signed-off-by: sunliming <sunliming@kylinos.cn>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
index 6e63df816d85..6dec2623d0be 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
@@ -172,6 +172,8 @@ r535_dmac_alloc(struct nvkm_disp *disp, u32 oclass, int inst, u32 put_offset,
 
 	args = nvkm_gsp_rm_alloc_get(&disp->rm.object, (oclass << 16) | inst, oclass,
 				     sizeof(*args), dmac);
+	if (!args)
+		return -ENOMEM;
 	if (IS_ERR(args))
 		return PTR_ERR(args);
 
-- 
2.25.1
Re: [PATCH] drm/nouveau/gsp: Fix possible NULL pointer dereference warning in r535_dmac_alloc
Posted by Danilo Krummrich 1 month, 4 weeks ago
On Sat Apr 18, 2026 at 9:14 AM CEST, sunliming wrote:
> From: sunliming <sunliming@kylinos.cn>
>
> Fix below smatch warnings:
> drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c:178 r535_dmac_alloc()
> warn: 'args' can also be NULL

Please write a commit description that explains also the underlying problem; see
more below.

> Signed-off-by: sunliming <sunliming@kylinos.cn>
> ---
>  drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
> index 6e63df816d85..6dec2623d0be 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
> @@ -172,6 +172,8 @@ r535_dmac_alloc(struct nvkm_disp *disp, u32 oclass, int inst, u32 put_offset,
>  
>  	args = nvkm_gsp_rm_alloc_get(&disp->rm.object, (oclass << 16) | inst, oclass,
>  				     sizeof(*args), dmac);
> +	if (!args)
> +		return -ENOMEM;

Are we sure that this can ever return NULL in the first place? I know that
nvkm_gsp_rm_alloc_get() internally checks for IS_ERR_OR_NULL(), but I couldn't
find anything within the callchain that would actually return NULL.

That said, I think IS_ERR_OR_NULL() checks are misleading.

>  	if (IS_ERR(args))
>  		return PTR_ERR(args);
>  
> -- 
> 2.25.1
Re: [PATCH] drm/nouveau/gsp: Fix possible NULL pointer dereference warning in r535_dmac_alloc
Posted by sunliming@linux.dev 1 month, 3 weeks ago
2026年4月18日 20:12, "Danilo Krummrich" <dakr@kernel.org mailto:dakr@kernel.org?to=%22Danilo%20Krummrich%22%20%3Cdakr%40kernel.org%3E > 写到:


> 
> On Sat Apr 18, 2026 at 9:14 AM CEST, sunliming wrote:
> 
> > 
> > From: sunliming <sunliming@kylinos.cn>
> > 
> >  Fix below smatch warnings:
> >  drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c:178 r535_dmac_alloc()
> >  warn: 'args' can also be NULL
> > 
> Please write a commit description that explains also the underlying problem; see
> more below.
> 
> > 
> > Signed-off-by: sunliming <sunliming@kylinos.cn>
> >  ---
> >  drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> >  diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
> >  index 6e63df816d85..6dec2623d0be 100644
> >  --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
> >  +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
> >  @@ -172,6 +172,8 @@ r535_dmac_alloc(struct nvkm_disp *disp, u32 oclass, int inst, u32 put_offset,
> >  
> >  args = nvkm_gsp_rm_alloc_get(&disp->rm.object, (oclass << 16) | inst, oclass,
> >  sizeof(*args), dmac);
> >  + if (!args)
> >  + return -ENOMEM;
> > 
> Are we sure that this can ever return NULL in the first place? I know that
> nvkm_gsp_rm_alloc_get() internally checks for IS_ERR_OR_NULL(), but I couldn't
> find anything within the callchain that would actually return NULL.
> 
> That said, I think IS_ERR_OR_NULL() checks are misleading.
> 
I also believe that the nvkm_gsp_rm_alloc_get function will not return NULL. My patch 
was merely intended to suppress a compiler warning and to guard against possible future 
changes in the implementation of nvkm_gsp_rm_alloc_get. Of course, the patch may be 
unnecessary, and I will defer to the maintainers' review conclusion.

Thanks.
> > 
> > if (IS_ERR(args))
> >  return PTR_ERR(args);
> >  
> >  -- 
> >  2.25.1
> >
>
Re: [PATCH] drm/nouveau/gsp: Fix possible NULL pointer dereference warning in r535_dmac_alloc
Posted by Timur Tabi 1 month, 4 weeks ago
On Sat, 2026-04-18 at 14:12 +0200, Danilo Krummrich wrote:
> Are we sure that this can ever return NULL in the first place? I know that
> nvkm_gsp_rm_alloc_get() internally checks for IS_ERR_OR_NULL(), but I couldn't
> find anything within the callchain that would actually return NULL.
> 
> That said, I think IS_ERR_OR_NULL() checks are misleading.

I posted a patch a couple months ago that cleans up an IS_ERR check, but it was never picked up:

https://lore.freedesktop.org/nouveau/20260103000756.1002890-1-ttabi@nvidia.com/T/#u

I'm quite certain that there are other similar issues in Nouveau w.r.t. IS_ERR_OR_NULL.

I think an audit of the Nouveau error paths is in order.  I suspect that Ben's big refactors
over the past year have resulted in some invalid error paths.

Re: [PATCH] drm/nouveau/gsp: Fix possible NULL pointer dereference warning in r535_dmac_alloc
Posted by Danilo Krummrich 1 month, 4 weeks ago
On Sat Apr 18, 2026 at 7:24 PM CEST, Timur Tabi wrote:
> On Sat, 2026-04-18 at 14:12 +0200, Danilo Krummrich wrote:
>> Are we sure that this can ever return NULL in the first place? I know that
>> nvkm_gsp_rm_alloc_get() internally checks for IS_ERR_OR_NULL(), but I couldn't
>> find anything within the callchain that would actually return NULL.
>> 
>> That said, I think IS_ERR_OR_NULL() checks are misleading.
>
> I posted a patch a couple months ago that cleans up an IS_ERR check, but it was never picked up:

Thanks! Can you please make sure to send patches to all maintainers? If they
don't land in my inbox, I can't pick them up. :(

> https://lore.freedesktop.org/nouveau/20260103000756.1002890-1-ttabi@nvidia.com/T/#u
>
> I'm quite certain that there are other similar issues in Nouveau w.r.t. IS_ERR_OR_NULL.

Yeah, that sounds likely.

> I think an audit of the Nouveau error paths is in order.  I suspect that Ben's big refactors
> over the past year have resulted in some invalid error paths.

Are you planning to look into this?

Thanks,
Danilo
Re: [PATCH] drm/nouveau/gsp: Fix possible NULL pointer dereference warning in r535_dmac_alloc
Posted by Timur Tabi 1 month, 4 weeks ago
On Sat, 2026-04-18 at 19:35 +0200, Danilo Krummrich wrote:
> > I posted a patch a couple months ago that cleans up an IS_ERR check, but it was never picked
> > up:
> 
> Thanks! Can you please make sure to send patches to all maintainers? If they
> don't land in my inbox, I can't pick them up. :(

M:  Lyude Paul <lyude@redhat.com>
M:  Danilo Krummrich <dakr@kernel.org>

Hmmmm.... I did CC Lyude, but you're right.  I should update my git-email script to catch these
better.

While I have your attention, I do have other patches that I neglected to CC you on:

  2. [PATCH 6/6] drm/nouveau/gsp: enable FWSEC-SB on GA100
     - by Timur Tabi @ 2026-04-07 19:21 UTC [95%]

  3. [PATCH 2/6] drm/nouveau/bios: specify correct display fuse register for Ampere and Ada
     - by Timur Tabi @ 2026-04-07 19:21 UTC [95%]

  4. [PATCH 1/6] drm/nouveau: check for GA100 specifically when calculating FRTS size
     - by Timur Tabi @ 2026-04-07 19:21 UTC [91%]

  5. [PATCH 4/6] drm/nouveau/gsp: require GSP-RM for GA100 support
     - by Timur Tabi @ 2026-04-07 19:21 UTC [97%]

  6. [PATCH 3/6] drm/nouveau/bios: skip the IFR header if present
     - by Timur Tabi @ 2026-04-07 19:21 UTC [75%]

  7. [PATCH 5/6] drm/nouveau: parse the VBIOS on GA100
     - by Timur Tabi @ 2026-04-07 19:21 UTC [99%]

  8. [PATCH 0/6] drm/nouveau: fix GA100 issues
     - by Timur Tabi @ 2026-04-07 19:21 UTC [93%]

  9. [PATCH] drm/nouveau: expose VBIOS via debugfs on GSP-RM systems
     - by Timur Tabi @ 2026-04-07 18:53 UTC [95%]

> 
> > https://lore.freedesktop.org/nouveau/20260103000756.1002890-1-ttabi@nvidia.com/T/#u
> > 
> > I'm quite certain that there are other similar issues in Nouveau w.r.t. IS_ERR_OR_NULL.
> 
> Yeah, that sounds likely.
> 
> > I think an audit of the Nouveau error paths is in order.  I suspect that Ben's big refactors
> > over the past year have resulted in some invalid error paths.
> 
> Are you planning to look into this?

No, but it does sound like something that Claude could figure out pretty quick.
Re: [PATCH] drm/nouveau/gsp: Fix possible NULL pointer dereference warning in r535_dmac_alloc
Posted by Danilo Krummrich 1 month, 4 weeks ago
On 4/18/26 7:44 PM, Timur Tabi wrote:
> On Sat, 2026-04-18 at 19:35 +0200, Danilo Krummrich wrote:
>>> I posted a patch a couple months ago that cleans up an IS_ERR check, but it was never picked
>>> up:
>>
>> Thanks! Can you please make sure to send patches to all maintainers? If they
>> don't land in my inbox, I can't pick them up. :(
> 
> M:  Lyude Paul <lyude@redhat.com>
> M:  Danilo Krummrich <dakr@kernel.org>
> 
> Hmmmm.... I did CC Lyude, but you're right.  I should update my git-email script to catch these
> better.
> 
> While I have your attention, I do have other patches that I neglected to CC you on:
> 
>   2. [PATCH 6/6] drm/nouveau/gsp: enable FWSEC-SB on GA100
>      - by Timur Tabi @ 2026-04-07 19:21 UTC [95%]
> 
>   3. [PATCH 2/6] drm/nouveau/bios: specify correct display fuse register for Ampere and Ada
>      - by Timur Tabi @ 2026-04-07 19:21 UTC [95%]
> 
>   4. [PATCH 1/6] drm/nouveau: check for GA100 specifically when calculating FRTS size
>      - by Timur Tabi @ 2026-04-07 19:21 UTC [91%]
> 
>   5. [PATCH 4/6] drm/nouveau/gsp: require GSP-RM for GA100 support
>      - by Timur Tabi @ 2026-04-07 19:21 UTC [97%]
> 
>   6. [PATCH 3/6] drm/nouveau/bios: skip the IFR header if present
>      - by Timur Tabi @ 2026-04-07 19:21 UTC [75%]
> 
>   7. [PATCH 5/6] drm/nouveau: parse the VBIOS on GA100
>      - by Timur Tabi @ 2026-04-07 19:21 UTC [99%]
> 
>   8. [PATCH 0/6] drm/nouveau: fix GA100 issues
>      - by Timur Tabi @ 2026-04-07 19:21 UTC [93%]
> 
>   9. [PATCH] drm/nouveau: expose VBIOS via debugfs on GSP-RM systems
>      - by Timur Tabi @ 2026-04-07 18:53 UTC [95%]

Ok, I will take a look in the next days.

Thanks,
Danilo

>>> https://lore.freedesktop.org/nouveau/20260103000756.1002890-1-ttabi@nvidia.com/T/#u
>>>
>>> I'm quite certain that there are other similar issues in Nouveau w.r.t. IS_ERR_OR_NULL.
>>
>> Yeah, that sounds likely.
>>
>>> I think an audit of the Nouveau error paths is in order.  I suspect that Ben's big refactors
>>> over the past year have resulted in some invalid error paths.
>>
>> Are you planning to look into this?
> 
> No, but it does sound like something that Claude could figure out pretty quick.