[PATCH] nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc

[PATCH] nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc

[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

Re: [PATCH] nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc

[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/

Claude review: nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc

[Claude Code Review Bot]

Overall Series Review

Subject: nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc
Author: Hongling Zeng <zenghongling@kylinos.cn>
Patches: 2
Reviewed: 2026-05-27T15:18:37.432235

---

This is a single-patch fix for a potential NULL pointer dereference in the nouveau/gsp r535 nvenc and ofa allocation paths. The analysis is correct: `nvkm_gsp_rm_alloc_get()` calls through `gsp->rm->api->alloc->get()` which calls `nvkm_gsp_rpc_get()`, which in turn calls `gsp->rm->api->rpc->get()`. The `nvkm_gsp_rpc_get()` wrapper propagates whatever the underlying `.get` returns — and the sibling function `nvkm_gsp_rpc_rd()` already guards against NULL with `IS_ERR_OR_NULL()`. The `nvkm_gsp_rm_alloc_get()` inline in `gsp.h` also uses `IS_ERR_OR_NULL()` internally at line 376, and the `nvkm_gsp_rm_alloc()` convenience wrapper at line 418 does the same. So the callers that only check `IS_ERR()` are indeed inconsistent and vulnerable.

The fix is correct and matches the existing pattern in `nvkm_gsp_rm_alloc()`. However, it is **incomplete** — the same bug exists in several sibling files that are not touched by this patch.

---
Generated by Claude Code Patch Reviewer

Claude review: nouveau/gsp: fix NULL pointer dereference in r535 nvenc/ofs alloc

[Claude Code Review Bot]

Patch Review

**Correctness: The fix itself is correct.**

The change from:
```c
if (WARN_ON(IS_ERR(args)))
    return PTR_ERR(args);
```
to:
```c
if (WARN_ON(IS_ERR_OR_NULL(args)))
    return args ? PTR_ERR(args) : -EIO;
```

correctly mirrors the pattern established by `nvkm_gsp_rm_alloc()` in `gsp.h:418-419`:
```c
if (IS_ERR_OR_NULL(argv))
    return argv ? PTR_ERR(argv) : -EIO;
```

**Issue 1 (Medium): Incomplete fix — same bug in ce.c, nvdec.c, nvjpg.c, and r570/ofa.c**

The patch only fixes `r535/nvenc.c` and `r535/ofa.c`, but the exact same `IS_ERR()`-only pattern exists in:
- `r535/ce.c:34` — `if (WARN_ON(IS_ERR(args)))` / `return PTR_ERR(args);`
- `r535/nvdec.c:33` — identical pattern
- `r535/nvjpg.c:33` — identical pattern
- `r570/ofa.c:16` — identical pattern

Additionally, `r535/fifo.c:88` and `r535/device.c:70,124` have the same pattern with `nvkm_gsp_rm_alloc_get()` calls guarded only by `IS_ERR()`. If the fix is warranted for nvenc/ofa, it should be applied to all callers for consistency and to avoid the same class of bug.

**Issue 2 (Nit): Subject line says "ofs" — should be "ofa"**

The subject line reads `fix NULL pointer dereference in r535 nvenc/ofs alloc` but the file being fixed is `ofa.c` (Optical Flow Accelerator). This appears to be a typo: "ofs" should be "ofa".

**Issue 3 (Minor): Choice of `-EIO` for the NULL case**

Using `-EIO` for the NULL return matches the existing `nvkm_gsp_rm_alloc()` convention, so this is consistent. However, `-ENOMEM` might be more semantically appropriate since a NULL return from the underlying RPC get function likely means memory allocation failed. That said, since the existing convention uses `-EIO`, keeping it consistent is fine.

**Summary:** The fix is correct in what it does, but should be expanded to cover all callers of `nvkm_gsp_rm_alloc_get()` that have the same `IS_ERR()`-only check. The subject line typo ("ofs" → "ofa") should also be corrected.

---
Generated by Claude Code Patch Reviewer