All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc
@ 2026-05-26  1:47 Hongling Zeng
  2026-05-26 13:16   ` Danilo Krummrich
  0 siblings, 1 reply; 7+ messages in thread
From: Hongling Zeng @ 2026-05-26  1:47 UTC (permalink / raw)
  To: lyude, dakr, maarten.lankhorst, mripard, tzimmermann, airlied,
	simona, airlied, ttabi, bskeggs, dri-devel
  Cc: nouveau, linux-kernel, zhongling0719, Hongling Zeng

nvkm_gsp_rm_alloc_get() can return NULL as well as error pointers.
The current code only checks for error pointers with IS_ERR(), which
would lead to a NULL pointer dereference if NULL is returned.

Fix by using IS_ERR_OR_NULL() instead of IS_ERR(), matching the
pattern used in nvkm_gsp_rm_alloc().

Fixes: 7c2d25f1e408 ("drm/nouveau/gsp: add common code for engines/engine objects")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvenc.c | 4 ++--
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ofa.c   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvenc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvenc.c
index acb3ce8bb9de..a67cc65abfcf 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvenc.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvenc.c
@@ -30,8 +30,8 @@ r535_nvenc_alloc(struct nvkm_gsp_object *chan, u32 handle, u32 class, int inst,
 	NV_MSENC_ALLOCATION_PARAMETERS *args;
 
 	args = nvkm_gsp_rm_alloc_get(chan, handle, class, sizeof(*args), nvenc);
-	if (WARN_ON(IS_ERR(args)))
-		return PTR_ERR(args);
+	if (WARN_ON(IS_ERR_OR_NULL(args)))
+		return args ? PTR_ERR(args) : -EIO;
 
 	args->size = sizeof(*args);
 	args->engineInstance = inst;
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ofa.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ofa.c
index 2156808cba4f..6d3b554108f9 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ofa.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ofa.c
@@ -30,8 +30,8 @@ r535_ofa_alloc(struct nvkm_gsp_object *chan, u32 handle, u32 class, int inst,
 	NV_OFA_ALLOCATION_PARAMETERS *args;
 
 	args = nvkm_gsp_rm_alloc_get(chan, handle, class, sizeof(*args), ofa);
-	if (WARN_ON(IS_ERR(args)))
-		return PTR_ERR(args);
+	if (WARN_ON(IS_ERR_OR_NULL(args)))
+		return args ? PTR_ERR(args) : -EIO;
 
 	args->size = sizeof(*args);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc
  2026-05-26  1:47 [PATCH] nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc Hongling Zeng
@ 2026-05-26 13:16   ` Danilo Krummrich
  0 siblings, 0 replies; 7+ messages in thread
From: Danilo Krummrich @ 2026-05-26 13:16 UTC (permalink / raw)
  To: Hongling Zeng
  Cc: maarten.lankhorst, mripard, simona, airlied, bskeggs, dri-devel,
	nouveau, linux-kernel, zhongling0719

On Tue May 26, 2026 at 3:47 AM CEST, Hongling Zeng wrote:
> nvkm_gsp_rm_alloc_get() can return NULL as well as error pointers.
> The current code only checks for error pointers with IS_ERR(), which
> would lead to a NULL pointer dereference if NULL is returned.
>
> Fix by using IS_ERR_OR_NULL() instead of IS_ERR(), matching the
> pattern used in nvkm_gsp_rm_alloc().

There was a similar patch [1] a while ago for another callsite. I replied:

	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.

Is there a real case where NULL can be returned? If not, let's remove the
IS_ERR_OR_NULL() throughout the whole chain instead.

[1] https://lore.kernel.org/lkml/20260418071412.86022-1-sunliming@linux.dev/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc
@ 2026-05-26 13:16   ` Danilo Krummrich
  0 siblings, 0 replies; 7+ messages in thread
From: Danilo Krummrich @ 2026-05-26 13:16 UTC (permalink / raw)
  To: Hongling Zeng
  Cc: lyude, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
	airlied, ttabi, bskeggs, dri-devel, nouveau, linux-kernel,
	zhongling0719

On Tue May 26, 2026 at 3:47 AM CEST, Hongling Zeng wrote:
> nvkm_gsp_rm_alloc_get() can return NULL as well as error pointers.
> The current code only checks for error pointers with IS_ERR(), which
> would lead to a NULL pointer dereference if NULL is returned.
>
> Fix by using IS_ERR_OR_NULL() instead of IS_ERR(), matching the
> pattern used in nvkm_gsp_rm_alloc().

There was a similar patch [1] a while ago for another callsite. I replied:

	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.

Is there a real case where NULL can be returned? If not, let's remove the
IS_ERR_OR_NULL() throughout the whole chain instead.

[1] https://lore.kernel.org/lkml/20260418071412.86022-1-sunliming@linux.dev/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc
  2026-05-26 13:16   ` Danilo Krummrich
@ 2026-05-27  1:56     ` Hongling Zeng
  -1 siblings, 0 replies; 7+ messages in thread
From: Hongling Zeng @ 2026-05-27  1:56 UTC (permalink / raw)
  To: Danilo Krummrich, Hongling Zeng
  Cc: lyude, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
	airlied, ttabi, bskeggs, dri-devel, nouveau, linux-kernel

   Hi Danilo,

Thank you for the feedback. You're right.

  After tracing through the call chain:
nvkm_gsp_rm_alloc_get()
     └─> r535_gsp_rpc_rm_alloc_get()
         └─> r535_gsp_rpc_get()
             └─> r535_gsp_cmdq_get()
                 └─> kvzalloc()

  r535_gsp_cmdq_get() returns ERR_PTR(-ENOMEM)
   on allocation failure, not NULL. So NULL is never actually returned.

   I found a similar issue in sunrpc where IS_ERR_OR_NULL() is actively 
harmful -
   PTR_ERR(NULL) would return 0 (EOF), masking real errors. This 
confirms the pattern
   you identified.

   Should I submit a patch to clean up the IS_ERR_OR_NULL() checks in:
   - nvkm_gsp_rm_alloc_get() / nvkm_gsp_rm_alloc()
   - nvkm_gsp_rpc_rd()
   - All the callers

   Or would you prefer to handle this differently?

   Regards,
   Hongling


在 2026年05月26日 21:16, Danilo Krummrich 写道:
> On Tue May 26, 2026 at 3:47 AM CEST, Hongling Zeng wrote:
>> nvkm_gsp_rm_alloc_get() can return NULL as well as error pointers.
>> The current code only checks for error pointers with IS_ERR(), which
>> would lead to a NULL pointer dereference if NULL is returned.
>>
>> Fix by using IS_ERR_OR_NULL() instead of IS_ERR(), matching the
>> pattern used in nvkm_gsp_rm_alloc().
> There was a similar patch [1] a while ago for another callsite. I replied:
>
> 	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.
>
> Is there a real case where NULL can be returned? If not, let's remove the
> IS_ERR_OR_NULL() throughout the whole chain instead.
>
> [1] https://lore.kernel.org/lkml/20260418071412.86022-1-sunliming@linux.dev/


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc
@ 2026-05-27  1:56     ` Hongling Zeng
  0 siblings, 0 replies; 7+ messages in thread
From: Hongling Zeng @ 2026-05-27  1:56 UTC (permalink / raw)
  To: Danilo Krummrich, Hongling Zeng
  Cc: maarten.lankhorst, mripard, simona, airlied, bskeggs, dri-devel,
	nouveau, linux-kernel

   Hi Danilo,

Thank you for the feedback. You're right.

  After tracing through the call chain:
nvkm_gsp_rm_alloc_get()
     └─> r535_gsp_rpc_rm_alloc_get()
         └─> r535_gsp_rpc_get()
             └─> r535_gsp_cmdq_get()
                 └─> kvzalloc()

  r535_gsp_cmdq_get() returns ERR_PTR(-ENOMEM)
   on allocation failure, not NULL. So NULL is never actually returned.

   I found a similar issue in sunrpc where IS_ERR_OR_NULL() is actively 
harmful -
   PTR_ERR(NULL) would return 0 (EOF), masking real errors. This 
confirms the pattern
   you identified.

   Should I submit a patch to clean up the IS_ERR_OR_NULL() checks in:
   - nvkm_gsp_rm_alloc_get() / nvkm_gsp_rm_alloc()
   - nvkm_gsp_rpc_rd()
   - All the callers

   Or would you prefer to handle this differently?

   Regards,
   Hongling


在 2026年05月26日 21:16, Danilo Krummrich 写道:
> On Tue May 26, 2026 at 3:47 AM CEST, Hongling Zeng wrote:
>> nvkm_gsp_rm_alloc_get() can return NULL as well as error pointers.
>> The current code only checks for error pointers with IS_ERR(), which
>> would lead to a NULL pointer dereference if NULL is returned.
>>
>> Fix by using IS_ERR_OR_NULL() instead of IS_ERR(), matching the
>> pattern used in nvkm_gsp_rm_alloc().
> There was a similar patch [1] a while ago for another callsite. I replied:
>
> 	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.
>
> Is there a real case where NULL can be returned? If not, let's remove the
> IS_ERR_OR_NULL() throughout the whole chain instead.
>
> [1] https://lore.kernel.org/lkml/20260418071412.86022-1-sunliming@linux.dev/


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc
  2026-05-27  1:56     ` Hongling Zeng
@ 2026-05-27 10:39       ` Danilo Krummrich
  -1 siblings, 0 replies; 7+ messages in thread
From: Danilo Krummrich @ 2026-05-27 10:39 UTC (permalink / raw)
  To: Hongling Zeng
  Cc: Hongling Zeng, maarten.lankhorst, mripard, simona, airlied,
	bskeggs, dri-devel, nouveau, linux-kernel

On Wed May 27, 2026 at 3:56 AM CEST, Hongling Zeng wrote:
>    Should I submit a patch to clean up the IS_ERR_OR_NULL() checks in:
>    - nvkm_gsp_rm_alloc_get() / nvkm_gsp_rm_alloc()
>    - nvkm_gsp_rpc_rd()
>    - All the callers

Sounds good. Ideally, where it makes sense, do it in separate patches and send a
series.

Thanks,
Danilo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc
@ 2026-05-27 10:39       ` Danilo Krummrich
  0 siblings, 0 replies; 7+ messages in thread
From: Danilo Krummrich @ 2026-05-27 10:39 UTC (permalink / raw)
  To: Hongling Zeng
  Cc: Hongling Zeng, lyude, maarten.lankhorst, mripard, tzimmermann,
	airlied, simona, airlied, ttabi, bskeggs, dri-devel, nouveau,
	linux-kernel

On Wed May 27, 2026 at 3:56 AM CEST, Hongling Zeng wrote:
>    Should I submit a patch to clean up the IS_ERR_OR_NULL() checks in:
>    - nvkm_gsp_rm_alloc_get() / nvkm_gsp_rm_alloc()
>    - nvkm_gsp_rpc_rd()
>    - All the callers

Sounds good. Ideally, where it makes sense, do it in separate patches and send a
series.

Thanks,
Danilo

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-05-27 15:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-26  1:47 [PATCH] nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc Hongling Zeng
2026-05-26 13:16 ` Danilo Krummrich
2026-05-26 13:16   ` Danilo Krummrich
2026-05-27  1:56   ` Hongling Zeng
2026-05-27  1:56     ` Hongling Zeng
2026-05-27 10:39     ` Danilo Krummrich
2026-05-27 10:39       ` Danilo Krummrich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.