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(-)
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
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/
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/
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
© 2016 - 2026 Red Hat, Inc.