[PATCH v5 0/5] nouveau/gsp: Clean up IS_ERR vs IS_ERR_OR_NULL usage

[PATCH v5 0/5] nouveau/gsp: Clean up IS_ERR vs IS_ERR_OR_NULL usage

[Hongling Zeng]

This series addresses feedback from Danilo Krummrich and Timur Tabi
regarding the use of IS_ERR() vs IS_ERR_OR_NULL() in the nouveau
GSP RPC code.

The key insight is that we should align error checking with the actual
return value contracts of each function:
- Functions that never return NULL should use IS_ERR()
- Functions that can return NULL should use IS_ERR_OR_NULL()

This version adds documentation to clarify the return value contracts,
making it easier for maintainers to validate future changes.

Return Value Analysis

After thorough code analysis, the RPC functions are categorized as:

Never return NULL (use IS_ERR):**
- r535_gsp_msgq_peek()
- r535_gsp_msgq_recv_one_elem()
- r535_gsp_rpc_get()

CAN return NULL (use IS_ERR_OR_NULL):**
- r535_gsp_msgq_recv() - returns NULL when RPC length is invalid
- r535_gsp_msg_recv() - returns NULL when queue drained
- r535_gsp_rpc_handle_reply() - returns NULL for NOWAIT/NOSEQ policies
- r535_gsp_rpc_send() - can return NULL via handle_reply
- r535_gsp_rpc_push() - can return NULL via handle_reply

Changes in v2

- Added kernel-doc comments documenting return value contracts for all
  RPC functions
- Cleaned up incorrect IS_ERR_OR_NULL() usage for functions that never
  return NULL
- Kept IS_ERR_OR_NULL() in places where NULL is actually possible
- Added Fixes tags to all patches

Testing

This fixes the NULL pointer dereference oops that occurs during
nouveau initialization on some systems.

Patches

Hongling Zeng (5):
  nouveau/gsp/rpc: Document RPC function return value contracts
  nouveau/gsp/rpc: Cleanup incorrect IS_ERR_OR_NULL in rpc.c
  nouveau/gsp/rm/alloc: Cleanup IS_ERR_OR_NULL usage
  nouveau/gsp/rm/bar: Cleanup IS_ERR_OR_NULL usage
  nouveau/gsp: Cleanup IS_ERR_OR_NULL in nvkm_gsp_rpc_rd()

 drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h   | 2 +-
 .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c | 2 +-
 .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c  | 2 +-
 .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c  | 81 ++++++++++++++++++-
 4 files changed, 81 insertions(+), 6 deletions(-)

[PATCH v5 1/5] nouveau/gsp/rpc: Document RPC function return value contracts

[Hongling Zeng]

Add kernel-doc comments to document the return value semantics of
RPC functions in r535/rpc.c. This clarifies which functions can return
NULL, error pointers, or both, helping maintainers validate future
changes.

Return value analysis:
- r535_gsp_msgq_peek(): Never returns NULL
- r535_gsp_msgq_recv_one_elem(): Never returns NULL
- r535_gsp_msgq_recv(): CAN return NULL (when RPC length invalid)
- r535_gsp_msg_recv(): CAN return NULL (queue drained/no matching message)
- r535_gsp_rpc_get(): Never returns NULL
- r535_gsp_rpc_handle_reply(): CAN return NULL (NOWAIT/NOSEQ policies)
- r535_gsp_rpc_send(): CAN return NULL (via handle_reply)
- r535_gsp_rpc_push(): CAN return NULL (via handle_reply)

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

---
 Change in v5:
 -Fix documentation accuracy for r535_gsp_msg_recv() to clarify.
 -Fix documentation accuracy for r535_gsp_rpc_handle_reply() to clarify.
---
 .../drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c | 87 +++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
index 3ca3de8f4340..1f953f042df0 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
@@ -204,6 +204,15 @@ r535_gsp_msgq_get_entry(struct nvkm_gsp *gsp)
  *   The user is responsible for freeing the memory allocated for the GSP
  *   message pages after they have been processed.
  */
+/**
+ * r535_gsp_msgq_peek - Peek at next message in GSP command queue
+ * @gsp: GSP device
+ * @gsp_rpc_len: Expected RPC length
+ * @retries: Retry counter
+ *
+ * Return: Pointer to RPC message on success, or ERR_PTR() on error.
+ *         Never returns NULL.
+ */
 static void *
 r535_gsp_msgq_peek(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 {
@@ -229,6 +238,14 @@ struct r535_gsp_msg_info {
 static void
 r535_gsp_msg_dump(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc *msg, int lvl);
 
+/**
+ * r535_gsp_msgq_recv_one_elem - Receive one message element from GSP queue
+ * @gsp: GSP device
+ * @info: Message queue information
+ *
+ * Return: Pointer to received buffer on success, or ERR_PTR() on error.
+ *         Never returns NULL.
+ */
 static void *
 r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 			    struct r535_gsp_msg_info *info)
@@ -283,6 +300,15 @@ r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 	return buf;
 }
 
+/**
+ * r535_gsp_msgq_recv - Receive RPC message(s) from GSP queue
+ * @gsp: GSP device
+ * @gsp_rpc_len: Expected RPC length
+ * @retries: Retry counter
+ *
+ * Return: Pointer to received buffer on success, ERR_PTR() on error,
+ *         or NULL if RPC length is invalid.
+ */
 static void *
 r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 {
@@ -450,6 +476,20 @@ r535_gsp_msg_dump(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc *msg, int lvl)
 	}
 }
 
+/**
+ * r535_gsp_msg_recv - Receive RPC message from GSP message queue
+ * @gsp: GSP device
+ * @fn: Expected RPC function number (0 to accept any)
+ * @gsp_rpc_len: Expected RPC length (0 for POLL mode)
+ *
+ * Return: Pointer to RPC message on success, ERR_PTR() on error.
+ *         When fn!=0: retries until timeout, returns ERR_PTR(-ETIMEDOUT)
+ *                    if no match found (never returns NULL for timeout).
+ *                    When fn!=0 and gsp_rpc_len==0 (POLL mode): returns
+ *                    NULL on successful message processing.
+ *         When fn==0: returns NULL if queue is drained or no matching
+ *                    message found without retrying.
+ */
 struct nvfw_gsp_rpc *
 r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
 {
@@ -547,6 +587,17 @@ r535_gsp_rpc_poll(struct nvkm_gsp *gsp, u32 fn)
 	return 0;
 }
 
+/**
+ * r535_gsp_rpc_handle_reply - Handle RPC reply based on policy
+ * @gsp: GSP device
+ * @fn: RPC function number
+ * @policy: Reply policy (NOWAIT, NOSEQ, RECV, or POLL)
+ * @gsp_rpc_len: Expected RPC length
+ *
+ * Return: NULL when policy is NOWAIT or NOSEQ (no reply expected),
+ *         pointer to reply data on success, ERR_PTR() on error.
+ *         When policy is POLL: NULL on successful message processing.
+ */
 static void *
 r535_gsp_rpc_handle_reply(struct nvkm_gsp *gsp, u32 fn,
 			  enum nvkm_gsp_rpc_reply_policy policy,
@@ -574,6 +625,17 @@ r535_gsp_rpc_handle_reply(struct nvkm_gsp *gsp, u32 fn,
 	return repv;
 }
 
+/**
+ * r535_gsp_rpc_send - Send RPC message and handle reply
+ * @gsp: GSP device
+ * @payload: RPC payload to send
+ * @policy: Reply policy (NOWAIT, NOSEQ, RECV, or POLL)
+ * @gsp_rpc_len: Expected RPC length for reply
+ *
+ * Return: NULL if policy is NOWAIT/NOSEQ (no reply expected) or
+ *         POLL (successful message processing), pointer to reply data
+ *         on success, or ERR_PTR() on error.
+ */
 static void *
 r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *payload,
 		  enum nvkm_gsp_rpc_reply_policy policy, u32 gsp_rpc_len)
@@ -601,6 +663,11 @@ r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *payload,
 	return r535_gsp_rpc_handle_reply(gsp, fn, policy, gsp_rpc_len);
 }
 
+/**
+ * r535_gsp_rpc_done - Free an RPC message
+ * @gsp: GSP device
+ * @repv: Pointer to RPC payload data returned by r535_gsp_rpc_push()
+ */
 static void
 r535_gsp_rpc_done(struct nvkm_gsp *gsp, void *repv)
 {
@@ -609,6 +676,15 @@ r535_gsp_rpc_done(struct nvkm_gsp *gsp, void *repv)
 	r535_gsp_msg_done(gsp, rpc);
 }
 
+/**
+ * r535_gsp_rpc_get - Allocate and initialize an RPC message
+ * @gsp: GSP device
+ * @fn: RPC function number
+ * @payload_size: Size of the payload
+ *
+ * Return: Pointer to RPC payload data on success, or ERR_PTR() on error.
+ *     Never returns NULL. 
+ */
 static void *
 r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 payload_size)
 {
@@ -628,6 +704,17 @@ r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 payload_size)
 	return rpc->data;
 }
 
+/**
+ * r535_gsp_rpc_push - Push RPC message to GSP and wait for reply
+ * @gsp: GSP device
+ * @payload: RPC payload to send
+ * @policy: Reply policy (NOWAIT, NOSEQ, RECV, or POLL)
+ * @gsp_rpc_len: Expected RPC length in the reply
+ *
+ * Return: NULL when policy is NOWAIT/NOSEQ (no reply expected) or
+ *         POLL (successful message processing), pointer to reply data
+ *         on success, or ERR_PTR() on error.
+ */
 static void *
 r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload,
 		  enum nvkm_gsp_rpc_reply_policy policy, u32 gsp_rpc_len)
-- 
2.25.1

[PATCH v5 1/5] nouveau/gsp/rpc: Document RPC function return value contracts

[Hongling Zeng]

Add kernel-doc comments to document the return value semantics of
RPC functions in r535/rpc.c. This clarifies which functions can return
NULL, error pointers, or both, helping maintainers validate future
changes.

Return value analysis:
- r535_gsp_msgq_peek(): Never returns NULL
- r535_gsp_msgq_recv_one_elem(): Never returns NULL
- r535_gsp_msgq_recv(): CAN return NULL (when RPC length invalid)
- r535_gsp_msg_recv(): CAN return NULL (queue drained/no matching message)
- r535_gsp_rpc_get(): Never returns NULL
- r535_gsp_rpc_handle_reply(): CAN return NULL (NOWAIT/NOSEQ policies)
- r535_gsp_rpc_send(): CAN return NULL (via handle_reply)
- r535_gsp_rpc_push(): CAN return NULL (via handle_reply)

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

---
 Change in v5:
 -Fix documentation accuracy for r535_gsp_msg_recv() to clarify.
 -Fix documentation accuracy for r535_gsp_rpc_handle_reply() to clarify.
---
 .../drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c | 87 +++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
index 3ca3de8f4340..1f953f042df0 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
@@ -204,6 +204,15 @@ r535_gsp_msgq_get_entry(struct nvkm_gsp *gsp)
  *   The user is responsible for freeing the memory allocated for the GSP
  *   message pages after they have been processed.
  */
+/**
+ * r535_gsp_msgq_peek - Peek at next message in GSP command queue
+ * @gsp: GSP device
+ * @gsp_rpc_len: Expected RPC length
+ * @retries: Retry counter
+ *
+ * Return: Pointer to RPC message on success, or ERR_PTR() on error.
+ *         Never returns NULL.
+ */
 static void *
 r535_gsp_msgq_peek(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 {
@@ -229,6 +238,14 @@ struct r535_gsp_msg_info {
 static void
 r535_gsp_msg_dump(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc *msg, int lvl);
 
+/**
+ * r535_gsp_msgq_recv_one_elem - Receive one message element from GSP queue
+ * @gsp: GSP device
+ * @info: Message queue information
+ *
+ * Return: Pointer to received buffer on success, or ERR_PTR() on error.
+ *         Never returns NULL.
+ */
 static void *
 r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 			    struct r535_gsp_msg_info *info)
@@ -283,6 +300,15 @@ r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 	return buf;
 }
 
+/**
+ * r535_gsp_msgq_recv - Receive RPC message(s) from GSP queue
+ * @gsp: GSP device
+ * @gsp_rpc_len: Expected RPC length
+ * @retries: Retry counter
+ *
+ * Return: Pointer to received buffer on success, ERR_PTR() on error,
+ *         or NULL if RPC length is invalid.
+ */
 static void *
 r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 {
@@ -450,6 +476,20 @@ r535_gsp_msg_dump(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc *msg, int lvl)
 	}
 }
 
+/**
+ * r535_gsp_msg_recv - Receive RPC message from GSP message queue
+ * @gsp: GSP device
+ * @fn: Expected RPC function number (0 to accept any)
+ * @gsp_rpc_len: Expected RPC length (0 for POLL mode)
+ *
+ * Return: Pointer to RPC message on success, ERR_PTR() on error.
+ *         When fn!=0: retries until timeout, returns ERR_PTR(-ETIMEDOUT)
+ *                    if no match found (never returns NULL for timeout).
+ *                    When fn!=0 and gsp_rpc_len==0 (POLL mode): returns
+ *                    NULL on successful message processing.
+ *         When fn==0: returns NULL if queue is drained or no matching
+ *                    message found without retrying.
+ */
 struct nvfw_gsp_rpc *
 r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
 {
@@ -547,6 +587,17 @@ r535_gsp_rpc_poll(struct nvkm_gsp *gsp, u32 fn)
 	return 0;
 }
 
+/**
+ * r535_gsp_rpc_handle_reply - Handle RPC reply based on policy
+ * @gsp: GSP device
+ * @fn: RPC function number
+ * @policy: Reply policy (NOWAIT, NOSEQ, RECV, or POLL)
+ * @gsp_rpc_len: Expected RPC length
+ *
+ * Return: NULL when policy is NOWAIT or NOSEQ (no reply expected),
+ *         pointer to reply data on success, ERR_PTR() on error.
+ *         When policy is POLL: NULL on successful message processing.
+ */
 static void *
 r535_gsp_rpc_handle_reply(struct nvkm_gsp *gsp, u32 fn,
 			  enum nvkm_gsp_rpc_reply_policy policy,
@@ -574,6 +625,17 @@ r535_gsp_rpc_handle_reply(struct nvkm_gsp *gsp, u32 fn,
 	return repv;
 }
 
+/**
+ * r535_gsp_rpc_send - Send RPC message and handle reply
+ * @gsp: GSP device
+ * @payload: RPC payload to send
+ * @policy: Reply policy (NOWAIT, NOSEQ, RECV, or POLL)
+ * @gsp_rpc_len: Expected RPC length for reply
+ *
+ * Return: NULL if policy is NOWAIT/NOSEQ (no reply expected) or
+ *         POLL (successful message processing), pointer to reply data
+ *         on success, or ERR_PTR() on error.
+ */
 static void *
 r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *payload,
 		  enum nvkm_gsp_rpc_reply_policy policy, u32 gsp_rpc_len)
@@ -601,6 +663,11 @@ r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *payload,
 	return r535_gsp_rpc_handle_reply(gsp, fn, policy, gsp_rpc_len);
 }
 
+/**
+ * r535_gsp_rpc_done - Free an RPC message
+ * @gsp: GSP device
+ * @repv: Pointer to RPC payload data returned by r535_gsp_rpc_push()
+ */
 static void
 r535_gsp_rpc_done(struct nvkm_gsp *gsp, void *repv)
 {
@@ -609,6 +676,15 @@ r535_gsp_rpc_done(struct nvkm_gsp *gsp, void *repv)
 	r535_gsp_msg_done(gsp, rpc);
 }
 
+/**
+ * r535_gsp_rpc_get - Allocate and initialize an RPC message
+ * @gsp: GSP device
+ * @fn: RPC function number
+ * @payload_size: Size of the payload
+ *
+ * Return: Pointer to RPC payload data on success, or ERR_PTR() on error.
+ *     Never returns NULL. 
+ */
 static void *
 r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 payload_size)
 {
@@ -628,6 +704,17 @@ r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 payload_size)
 	return rpc->data;
 }
 
+/**
+ * r535_gsp_rpc_push - Push RPC message to GSP and wait for reply
+ * @gsp: GSP device
+ * @payload: RPC payload to send
+ * @policy: Reply policy (NOWAIT, NOSEQ, RECV, or POLL)
+ * @gsp_rpc_len: Expected RPC length in the reply
+ *
+ * Return: NULL when policy is NOWAIT/NOSEQ (no reply expected) or
+ *         POLL (successful message processing), pointer to reply data
+ *         on success, or ERR_PTR() on error.
+ */
 static void *
 r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload,
 		  enum nvkm_gsp_rpc_reply_policy policy, u32 gsp_rpc_len)
-- 
2.25.1

[PATCH v5 1/5] nouveau/gsp/rpc: Document RPC function return value contracts

[Hongling Zeng]

Add kernel-doc comments to document the return value semantics of
RPC functions in r535/rpc.c. This clarifies which functions can return
NULL, error pointers, or both, helping maintainers validate future
changes.

Return value analysis:
- r535_gsp_msgq_peek(): Never returns NULL
- r535_gsp_msgq_recv_one_elem(): Never returns NULL
- r535_gsp_msgq_recv(): CAN return NULL (when RPC length invalid)
- r535_gsp_msg_recv(): CAN return NULL (queue drained/no matching message)
- r535_gsp_rpc_get(): Never returns NULL
- r535_gsp_rpc_handle_reply(): CAN return NULL (NOWAIT/NOSEQ policies)
- r535_gsp_rpc_send(): CAN return NULL (via handle_reply)
- r535_gsp_rpc_push(): CAN return NULL (via handle_reply)

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

---
 Change in v5:
 -Fix documentation accuracy for r535_gsp_msg_recv() to clarify.
 -Fix documentation accuracy for r535_gsp_rpc_handle_reply() to clarify.
---
 .../drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c | 87 +++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
index 3ca3de8f4340..1f953f042df0 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
@@ -204,6 +204,15 @@ r535_gsp_msgq_get_entry(struct nvkm_gsp *gsp)
  *   The user is responsible for freeing the memory allocated for the GSP
  *   message pages after they have been processed.
  */
+/**
+ * r535_gsp_msgq_peek - Peek at next message in GSP command queue
+ * @gsp: GSP device
+ * @gsp_rpc_len: Expected RPC length
+ * @retries: Retry counter
+ *
+ * Return: Pointer to RPC message on success, or ERR_PTR() on error.
+ *         Never returns NULL.
+ */
 static void *
 r535_gsp_msgq_peek(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 {
@@ -229,6 +238,14 @@ struct r535_gsp_msg_info {
 static void
 r535_gsp_msg_dump(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc *msg, int lvl);
 
+/**
+ * r535_gsp_msgq_recv_one_elem - Receive one message element from GSP queue
+ * @gsp: GSP device
+ * @info: Message queue information
+ *
+ * Return: Pointer to received buffer on success, or ERR_PTR() on error.
+ *         Never returns NULL.
+ */
 static void *
 r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 			    struct r535_gsp_msg_info *info)
@@ -283,6 +300,15 @@ r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 	return buf;
 }
 
+/**
+ * r535_gsp_msgq_recv - Receive RPC message(s) from GSP queue
+ * @gsp: GSP device
+ * @gsp_rpc_len: Expected RPC length
+ * @retries: Retry counter
+ *
+ * Return: Pointer to received buffer on success, ERR_PTR() on error,
+ *         or NULL if RPC length is invalid.
+ */
 static void *
 r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 {
@@ -450,6 +476,20 @@ r535_gsp_msg_dump(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc *msg, int lvl)
 	}
 }
 
+/**
+ * r535_gsp_msg_recv - Receive RPC message from GSP message queue
+ * @gsp: GSP device
+ * @fn: Expected RPC function number (0 to accept any)
+ * @gsp_rpc_len: Expected RPC length (0 for POLL mode)
+ *
+ * Return: Pointer to RPC message on success, ERR_PTR() on error.
+ *         When fn!=0: retries until timeout, returns ERR_PTR(-ETIMEDOUT)
+ *                    if no match found (never returns NULL for timeout).
+ *                    When fn!=0 and gsp_rpc_len==0 (POLL mode): returns
+ *                    NULL on successful message processing.
+ *         When fn==0: returns NULL if queue is drained or no matching
+ *                    message found without retrying.
+ */
 struct nvfw_gsp_rpc *
 r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
 {
@@ -547,6 +587,17 @@ r535_gsp_rpc_poll(struct nvkm_gsp *gsp, u32 fn)
 	return 0;
 }
 
+/**
+ * r535_gsp_rpc_handle_reply - Handle RPC reply based on policy
+ * @gsp: GSP device
+ * @fn: RPC function number
+ * @policy: Reply policy (NOWAIT, NOSEQ, RECV, or POLL)
+ * @gsp_rpc_len: Expected RPC length
+ *
+ * Return: NULL when policy is NOWAIT or NOSEQ (no reply expected),
+ *         pointer to reply data on success, ERR_PTR() on error.
+ *         When policy is POLL: NULL on successful message processing.
+ */
 static void *
 r535_gsp_rpc_handle_reply(struct nvkm_gsp *gsp, u32 fn,
 			  enum nvkm_gsp_rpc_reply_policy policy,
@@ -574,6 +625,17 @@ r535_gsp_rpc_handle_reply(struct nvkm_gsp *gsp, u32 fn,
 	return repv;
 }
 
+/**
+ * r535_gsp_rpc_send - Send RPC message and handle reply
+ * @gsp: GSP device
+ * @payload: RPC payload to send
+ * @policy: Reply policy (NOWAIT, NOSEQ, RECV, or POLL)
+ * @gsp_rpc_len: Expected RPC length for reply
+ *
+ * Return: NULL if policy is NOWAIT/NOSEQ (no reply expected) or
+ *         POLL (successful message processing), pointer to reply data
+ *         on success, or ERR_PTR() on error.
+ */
 static void *
 r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *payload,
 		  enum nvkm_gsp_rpc_reply_policy policy, u32 gsp_rpc_len)
@@ -601,6 +663,11 @@ r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *payload,
 	return r535_gsp_rpc_handle_reply(gsp, fn, policy, gsp_rpc_len);
 }
 
+/**
+ * r535_gsp_rpc_done - Free an RPC message
+ * @gsp: GSP device
+ * @repv: Pointer to RPC payload data returned by r535_gsp_rpc_push()
+ */
 static void
 r535_gsp_rpc_done(struct nvkm_gsp *gsp, void *repv)
 {
@@ -609,6 +676,15 @@ r535_gsp_rpc_done(struct nvkm_gsp *gsp, void *repv)
 	r535_gsp_msg_done(gsp, rpc);
 }
 
+/**
+ * r535_gsp_rpc_get - Allocate and initialize an RPC message
+ * @gsp: GSP device
+ * @fn: RPC function number
+ * @payload_size: Size of the payload
+ *
+ * Return: Pointer to RPC payload data on success, or ERR_PTR() on error.
+ *     Never returns NULL. 
+ */
 static void *
 r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 payload_size)
 {
@@ -628,6 +704,17 @@ r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 payload_size)
 	return rpc->data;
 }
 
+/**
+ * r535_gsp_rpc_push - Push RPC message to GSP and wait for reply
+ * @gsp: GSP device
+ * @payload: RPC payload to send
+ * @policy: Reply policy (NOWAIT, NOSEQ, RECV, or POLL)
+ * @gsp_rpc_len: Expected RPC length in the reply
+ *
+ * Return: NULL when policy is NOWAIT/NOSEQ (no reply expected) or
+ *         POLL (successful message processing), pointer to reply data
+ *         on success, or ERR_PTR() on error.
+ */
 static void *
 r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload,
 		  enum nvkm_gsp_rpc_reply_policy policy, u32 gsp_rpc_len)
-- 
2.25.1

[PATCH v5 2/5] nouveau/gsp/rpc: Cleanup incorrect IS_ERR_OR_NULL in rpc.c

[Hongling Zeng]

Clean up incorrect IS_ERR_OR_NULL() usage for functions that never
return NULL:
- r535_gsp_msgq_peek() never returns NULL
- r535_gsp_msgq_recv_one_elem() never returns NULL

These functions should be checked with IS_ERR() instead.

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
index 1f953f042df0..74d160cbb2b1 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
@@ -350,7 +350,7 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 		u32 size;
 
 		rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), info.retries);
-		if (IS_ERR_OR_NULL(rpc)) {
+		if (IS_ERR(rpc)) {
 			kvfree(buf);
 			return rpc;
 		}
@@ -359,7 +359,7 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 		info.continuation = true;
 
 		rpc = r535_gsp_msgq_recv_one_elem(gsp, &info);
-		if (IS_ERR_OR_NULL(rpc)) {
+		if (IS_ERR(rpc)) {
 			kvfree(buf);
 			return rpc;
 		}
@@ -499,7 +499,7 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
 
 retry:
 	rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), &retries);
-	if (IS_ERR_OR_NULL(rpc))
+	if (IS_ERR(rpc))
 		return rpc;
 
 	rpc = r535_gsp_msgq_recv(gsp, gsp_rpc_len, &retries);
-- 
2.25.1

[PATCH v5 2/5] nouveau/gsp/rpc: Cleanup incorrect IS_ERR_OR_NULL in rpc.c

[Hongling Zeng]

Clean up incorrect IS_ERR_OR_NULL() usage for functions that never
return NULL:
- r535_gsp_msgq_peek() never returns NULL
- r535_gsp_msgq_recv_one_elem() never returns NULL

These functions should be checked with IS_ERR() instead.

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
index 1f953f042df0..74d160cbb2b1 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
@@ -350,7 +350,7 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 		u32 size;
 
 		rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), info.retries);
-		if (IS_ERR_OR_NULL(rpc)) {
+		if (IS_ERR(rpc)) {
 			kvfree(buf);
 			return rpc;
 		}
@@ -359,7 +359,7 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 		info.continuation = true;
 
 		rpc = r535_gsp_msgq_recv_one_elem(gsp, &info);
-		if (IS_ERR_OR_NULL(rpc)) {
+		if (IS_ERR(rpc)) {
 			kvfree(buf);
 			return rpc;
 		}
@@ -499,7 +499,7 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
 
 retry:
 	rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), &retries);
-	if (IS_ERR_OR_NULL(rpc))
+	if (IS_ERR(rpc))
 		return rpc;
 
 	rpc = r535_gsp_msgq_recv(gsp, gsp_rpc_len, &retries);
-- 
2.25.1

[PATCH v5 2/5] nouveau/gsp/rpc: Cleanup incorrect IS_ERR_OR_NULL in rpc.c

[Hongling Zeng]

Clean up incorrect IS_ERR_OR_NULL() usage for functions that never
return NULL:
- r535_gsp_msgq_peek() never returns NULL
- r535_gsp_msgq_recv_one_elem() never returns NULL

These functions should be checked with IS_ERR() instead.

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
index 1f953f042df0..74d160cbb2b1 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
@@ -350,7 +350,7 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 		u32 size;
 
 		rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), info.retries);
-		if (IS_ERR_OR_NULL(rpc)) {
+		if (IS_ERR(rpc)) {
 			kvfree(buf);
 			return rpc;
 		}
@@ -359,7 +359,7 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 		info.continuation = true;
 
 		rpc = r535_gsp_msgq_recv_one_elem(gsp, &info);
-		if (IS_ERR_OR_NULL(rpc)) {
+		if (IS_ERR(rpc)) {
 			kvfree(buf);
 			return rpc;
 		}
@@ -499,7 +499,7 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
 
 retry:
 	rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), &retries);
-	if (IS_ERR_OR_NULL(rpc))
+	if (IS_ERR(rpc))
 		return rpc;
 
 	rpc = r535_gsp_msgq_recv(gsp, gsp_rpc_len, &retries);
-- 
2.25.1

[PATCH v5 3/5] nouveau/gsp/rm/alloc: Cleanup IS_ERR_OR_NULL usage

[Hongling Zeng]

r535_gsp_rpc_rm_free() calls nvkm_gsp_rpc_get() which never returns
NULL, only valid pointers or error pointers. Clean up by using IS_ERR()
instead, matching the actual return value contract.

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
index 46e3a29f2ad7..c9f86c0e9b25 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
@@ -35,7 +35,7 @@ r535_gsp_rpc_rm_free(struct nvkm_gsp_object *object)
 		   client->object.handle, object->handle);
 
 	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_FREE, sizeof(*rpc));
-	if (WARN_ON(IS_ERR_OR_NULL(rpc)))
+	if (WARN_ON(IS_ERR(rpc)))
 		return -EIO;
 
 	rpc->params.hRoot = client->object.handle;
-- 
2.25.1

[PATCH v5 3/5] nouveau/gsp/rm/alloc: Cleanup IS_ERR_OR_NULL usage

[Hongling Zeng]

r535_gsp_rpc_rm_free() calls nvkm_gsp_rpc_get() which never returns
NULL, only valid pointers or error pointers. Clean up by using IS_ERR()
instead, matching the actual return value contract.

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
index 46e3a29f2ad7..c9f86c0e9b25 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
@@ -35,7 +35,7 @@ r535_gsp_rpc_rm_free(struct nvkm_gsp_object *object)
 		   client->object.handle, object->handle);
 
 	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_FREE, sizeof(*rpc));
-	if (WARN_ON(IS_ERR_OR_NULL(rpc)))
+	if (WARN_ON(IS_ERR(rpc)))
 		return -EIO;
 
 	rpc->params.hRoot = client->object.handle;
-- 
2.25.1

[PATCH v5 4/5] nouveau/gsp/rm/bar: Cleanup IS_ERR_OR_NULL usage

[Hongling Zeng]

r535_bar_bar2_update_pde() calls nvkm_gsp_rpc_get() which never
returns NULL, only valid pointers or error pointers. Clean up by
using IS_ERR() instead, matching the actual return value contract.

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
index fae08ac3b18c..9cd68f8622d3 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
@@ -55,7 +55,7 @@ r535_bar_bar2_update_pde(struct nvkm_gsp *gsp, u8 page_shift, u64 pdbe)
 	rpc_update_bar_pde_v15_00 *rpc;
 
 	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_UPDATE_BAR_PDE, sizeof(*rpc));
-	if (WARN_ON(IS_ERR_OR_NULL(rpc)))
+	if (WARN_ON(IS_ERR(rpc)))
 		return -EIO;
 
 	rpc->info.barType = NV_RPC_UPDATE_PDE_BAR_2;
-- 
2.25.1

[PATCH v5 4/5] nouveau/gsp/rm/bar: Cleanup IS_ERR_OR_NULL usage

[Hongling Zeng]

r535_bar_bar2_update_pde() calls nvkm_gsp_rpc_get() which never
returns NULL, only valid pointers or error pointers. Clean up by
using IS_ERR() instead, matching the actual return value contract.

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
index fae08ac3b18c..9cd68f8622d3 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
@@ -55,7 +55,7 @@ r535_bar_bar2_update_pde(struct nvkm_gsp *gsp, u8 page_shift, u64 pdbe)
 	rpc_update_bar_pde_v15_00 *rpc;
 
 	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_UPDATE_BAR_PDE, sizeof(*rpc));
-	if (WARN_ON(IS_ERR_OR_NULL(rpc)))
+	if (WARN_ON(IS_ERR(rpc)))
 		return -EIO;
 
 	rpc->info.barType = NV_RPC_UPDATE_PDE_BAR_2;
-- 
2.25.1

[PATCH v5 5/5] nouveau/gsp: Cleanup IS_ERR_OR_NULL in nvkm_gsp_rpc_rd()

[Hongling Zeng]

nvkm_gsp_rpc_rd() calls nvkm_gsp_rpc_get() which never returns NULL,
only valid pointers or error pointers. Clean up by using IS_ERR()
instead, matching the actual return value contract.

Note: nvkm_gsp_rpc_push() can return NULL when policy is NOWAIT or
NOSEQ, but nvkm_gsp_rpc_rd() uses RECV policy which expects a reply.
If the RPC fails to get a reply, it should return an error pointer,
not NULL.

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
index 64fed208e4cf..a061779e0107 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
@@ -293,7 +293,7 @@ nvkm_gsp_rpc_rd(struct nvkm_gsp *gsp, u32 fn, u32 argc)
 {
 	void *argv = nvkm_gsp_rpc_get(gsp, fn, argc);
 
-	if (IS_ERR_OR_NULL(argv))
+	if (IS_ERR(argv))
 		return argv;
 
 	return nvkm_gsp_rpc_push(gsp, argv, NVKM_GSP_RPC_REPLY_RECV, argc);
-- 
2.25.1

[PATCH v5 5/5] nouveau/gsp: Cleanup IS_ERR_OR_NULL in nvkm_gsp_rpc_rd()

[Hongling Zeng]

nvkm_gsp_rpc_rd() calls nvkm_gsp_rpc_get() which never returns NULL,
only valid pointers or error pointers. Clean up by using IS_ERR()
instead, matching the actual return value contract.

Note: nvkm_gsp_rpc_push() can return NULL when policy is NOWAIT or
NOSEQ, but nvkm_gsp_rpc_rd() uses RECV policy which expects a reply.
If the RPC fails to get a reply, it should return an error pointer,
not NULL.

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
index 64fed208e4cf..a061779e0107 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
@@ -293,7 +293,7 @@ nvkm_gsp_rpc_rd(struct nvkm_gsp *gsp, u32 fn, u32 argc)
 {
 	void *argv = nvkm_gsp_rpc_get(gsp, fn, argc);
 
-	if (IS_ERR_OR_NULL(argv))
+	if (IS_ERR(argv))
 		return argv;
 
 	return nvkm_gsp_rpc_push(gsp, argv, NVKM_GSP_RPC_REPLY_RECV, argc);
-- 
2.25.1

[PATCH v5 5/5] nouveau/gsp: Cleanup IS_ERR_OR_NULL in nvkm_gsp_rpc_rd()

[Hongling Zeng]

nvkm_gsp_rpc_rd() calls nvkm_gsp_rpc_get() which never returns NULL,
only valid pointers or error pointers. Clean up by using IS_ERR()
instead, matching the actual return value contract.

Note: nvkm_gsp_rpc_push() can return NULL when policy is NOWAIT or
NOSEQ, but nvkm_gsp_rpc_rd() uses RECV policy which expects a reply.
If the RPC fails to get a reply, it should return an error pointer,
not NULL.

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
index 64fed208e4cf..a061779e0107 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
@@ -293,7 +293,7 @@ nvkm_gsp_rpc_rd(struct nvkm_gsp *gsp, u32 fn, u32 argc)
 {
 	void *argv = nvkm_gsp_rpc_get(gsp, fn, argc);
 
-	if (IS_ERR_OR_NULL(argv))
+	if (IS_ERR(argv))
 		return argv;
 
 	return nvkm_gsp_rpc_push(gsp, argv, NVKM_GSP_RPC_REPLY_RECV, argc);
-- 
2.25.1

Re: [PATCH v5 1/5] nouveau/gsp/rpc: Document RPC function return value contracts

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] RPC error path propagates NULL instead of ERR_PTR(), leading to silent failures and NULL pointer dereferences.
--

commit 79a7510afd6195ece62a51a8ba00ae15d75a0ac9
Author: Hongling Zeng <zenghongling@kylinos.cn>

nouveau/gsp/rpc: Document RPC function return value contracts

Add kernel-doc comments to document the return value semantics of
RPC functions in r535/rpc.c. This clarifies which functions can return
NULL, error pointers, or both.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
> index 3ca3de8f43408..b4b2b5c00ab4e 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c

[ ... ]

> @@ -283,6 +300,15 @@ r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
>  	return buf;
>  }
>  
> +/**
> + * r535_gsp_msgq_recv - Receive RPC message(s) from GSP queue
> + * @gsp: GSP device
> + * @gsp_rpc_len: Expected RPC length
> + * @retries: Retry counter
> + *
> + * Return: Pointer to received buffer on success, ERR_PTR() on error,
> + *         or NULL if RPC length is invalid.
> + */

[Severity: High]
This is a pre-existing issue, but does documenting NULL as a valid return
for an invalid RPC length mask an existing bug in the error path?

Currently, r535_gsp_msgq_recv() returns NULL when the length is invalid:

	if (WARN_ON(rpc->length > max_rpc_size))
		return NULL;

This NULL propagates up through r535_gsp_msg_recv() and 
r535_gsp_rpc_handle_reply() to callers like r535_gsp_get_static_info(),
which only checks for an error pointer:

	rpc = nvkm_gsp_rpc_rd(gsp, NV_VGPU_MSG_FUNCTION_GET_GSP_STATIC_INFO, sizeof(*rpc));
	if (IS_ERR(rpc))
		return PTR_ERR(rpc);
	
	gsp->internal.client.object.handle = rpc->hInternalClient;

Since IS_ERR(NULL) evaluates to false, this leads to an immediate NULL
pointer dereference.

In other cases like r535_gsp_rpc_rm_ctrl_push(), the error is silently
swallowed:

	rpc = nvkm_gsp_rpc_push(gsp, rpc, NVKM_GSP_RPC_REPLY_RECV, repc);
	if (IS_ERR_OR_NULL(rpc)) {
		*params = NULL;
		return PTR_ERR(rpc);
	}

Because PTR_ERR(NULL) evaluates to 0, the function incorrectly returns
success on a failed RPC message.

Should r535_gsp_msgq_recv() be changed to return an error pointer like
ERR_PTR(-EIO) instead of NULL, and the documentation updated to reflect
that failure mode?

>  static void *
>  r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
>  {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622081136.61695-1-zenghongling@kylinos.cn?part=1

Re: [PATCH v5 4/5] nouveau/gsp/rm/bar: Cleanup IS_ERR_OR_NULL usage

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Leak of the original RPC message buffer `msg` in error paths of `r535_gsp_rpc_push()` when `payload_size > max_payload_size`.
--

commit c9821a8788c2a50e0c5a5dcb65a00cbd271b7c0b
Author: Hongling Zeng <zenghongling@kylinos.cn>

nouveau/gsp/rm/bar: Cleanup IS_ERR_OR_NULL usage

r535_bar_bar2_update_pde() calls nvkm_gsp_rpc_get() which never
returns NULL, only valid pointers or error pointers. Clean up by
using IS_ERR() instead, matching the actual return value contract.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
> index fae08ac3b18c7..9cd68f8622d36 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
> @@ -55,7 +55,7 @@ r535_bar_bar2_update_pde(struct nvkm_gsp *gsp, u8 page_shift, u64 pdbe)
>  	rpc_update_bar_pde_v15_00 *rpc;
>  
>  	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_UPDATE_BAR_PDE, sizeof(*rpc));
> -	if (WARN_ON(IS_ERR_OR_NULL(rpc)))
> +	if (WARN_ON(IS_ERR(rpc)))
>  		return -EIO;

[Severity: High]
This is a pre-existing issue, but does r535_gsp_rpc_push() leak the original
RPC message buffer msg on error paths when payload_size > max_payload_size?

In drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c:r535_gsp_rpc_push(),
when the message is chunked, kvfree(msg) is called on the success path:

		next = r535_gsp_rpc_get(gsp, fn, max_payload_size);
		if (IS_ERR(next)) {
			repv = next;
			goto done;
		}
...
		repv = r535_gsp_rpc_handle_reply(gsp, fn, policy, payload_size +
						 sizeof(*rpc));
		if (!IS_ERR(repv))
			kvfree(msg);
	}
...
done:
	mutex_unlock(&gsp->cmdq.mutex);
	return repv;

If r535_gsp_rpc_get() fails, it jumps to done and returns without freeing
msg. Similarly, if r535_gsp_rpc_handle_reply() returns an error pointer,
msg is not freed because kvfree(msg) is guarded by !IS_ERR(repv).

Since these allocations can be large, can this cause a resource leak if the
RPC communications fail under load?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622081136.61695-1-zenghongling@kylinos.cn?part=4

Re: [PATCH v5 3/5] nouveau/gsp/rm/alloc: Cleanup IS_ERR_OR_NULL usage

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Silent failure and kernel panic when GSP RPC payload length exceeds max size
--

commit 3ec8c0b86229f7768cd026a5fe2006cc70c81806
Author: Hongling Zeng <zenghongling@kylinos.cn>

nouveau/gsp/rm/alloc: Cleanup IS_ERR_OR_NULL usage

r535_gsp_rpc_rm_free() calls nvkm_gsp_rpc_get() which never returns
NULL, only valid pointers or error pointers. Clean up by using IS_ERR()
instead, matching the actual return value contract.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
> index 46e3a29f2ad7f..c9f86c0e9b255 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
> @@ -35,7 +35,7 @@ r535_gsp_rpc_rm_free(struct nvkm_gsp_object *object)
>  		   client->object.handle, object->handle);
>  
>  	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_FREE, sizeof(*rpc));
> -	if (WARN_ON(IS_ERR_OR_NULL(rpc)))
> +	if (WARN_ON(IS_ERR(rpc)))
>  		return -EIO;

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this cleanup
of IS_ERR_OR_NULL(), I noticed a pre-existing issue where NULL pointers can
be returned and dereferenced in the GSP RPC receive paths.

If the GSP firmware returns an RPC message with an invalid length,
r535_gsp_msgq_recv() detects this but returns NULL instead of an error
pointer:

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c:r535_gsp_msgq_recv() {
        ...
        if (WARN_ON(rpc->length > max_rpc_size))
                return NULL;
        ...
}

This NULL propagates through functions like r535_gsp_rpc_rd(). Because callers
like r535_gsp_get_static_info() only check IS_ERR(), they will attempt to
dereference the NULL pointer:

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c:r535_gsp_get_static_info() {
        ...
        rpc = nvkm_gsp_rpc_rd(gsp, NV_VGPU_MSG_FUNCTION_GET_GSP_STATIC_INFO, sizeof(*rpc));
        if (IS_ERR(rpc))
                return PTR_ERR(rpc);

        gsp->internal.client.object.client = &gsp->internal.client;
        gsp->internal.client.object.parent = NULL;
        gsp->internal.client.object.handle = rpc->hInternalClient;
        ...
}

Could this NULL return also cause silent state corruption elsewhere? In
r535_gsp_rpc_rm_alloc_push(), a NULL return from nvkm_gsp_rpc_push() is
treated as a success condition:

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c:r535_gsp_rpc_rm_alloc_push() {
        ...
        rpc = nvkm_gsp_rpc_push(gsp, rpc, NVKM_GSP_RPC_REPLY_RECV, sizeof(*rpc));
        if (IS_ERR_OR_NULL(rpc))
                return rpc;
        ...
}

This means the length error could be silently converted into a false success,
leaving the object initialized in software but unallocated in hardware.

Should r535_gsp_msgq_recv() be updated to return an error pointer like
ERR_PTR(-EIO) instead of NULL to prevent these panics and false successes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622081136.61695-1-zenghongling@kylinos.cn?part=3

Re: [PATCH v5 2/5] nouveau/gsp/rpc: Cleanup incorrect IS_ERR_OR_NULL in rpc.c

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing upper and lower bounds checks on the untrusted `rpc->length` field from GSP messages causes severe integer underflows and heap buffer overflows.
- [High] Returning NULL on message length error in `r535_gsp_msgq_recv()` leads to false success reporting.
- [High] Early returns on error paths in GSP message reception fail to advance the read pointer, causing a permanently deadlocked queue.
- [High] Memory leak of large GSP RPC messages on error paths in `r535_gsp_rpc_push`.
- [High] Missing read memory barrier (`smp_rmb`) when polling the GSP message queue write pointer allows out-of-order reads of stale payload data.
--

commit b9953ccb9eee21566520494158c392c95237ec17
Author: Hongling Zeng <zenghongling@kylinos.cn>

nouveau/gsp/rpc: Cleanup incorrect IS_ERR_OR_NULL in rpc.c

This commit cleans up incorrect IS_ERR_OR_NULL() usage for functions
that never return NULL, such as r535_gsp_msgq_peek() and
r535_gsp_msgq_recv_one_elem(). It updates them to be checked with
IS_ERR() instead.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
> index b4b2b5c00ab4e..5b89c8835a2e4 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
> @@ -350,7 +350,7 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
>  		u32 size;
>  
>  		rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), info.retries);

[Severity: High]
This isn't a bug introduced by this patch, but earlier in r535_gsp_msgq_recv(),
if the message length is invalid, it returns NULL:

	if (WARN_ON(rpc->length > max_rpc_size))
		return NULL;

Since callers like nvkm_gsp_rm_alloc_wr() use IS_ERR() to check for failures,
does this cause the caller to incorrectly treat the operation as a success
because IS_ERR(NULL) evaluates to false?

[Severity: High]
This is a pre-existing issue, but do the early returns on error paths in
GSP message reception fail to advance the read pointer?

For example, in r535_gsp_msgq_recv():

	if (WARN_ON(rpc->length > max_rpc_size))
		return NULL;

And in r535_gsp_msgq_recv_one_elem():

	if (rpc->function != NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD) {
		nvkm_error(&gsp->subdev,
			   "Not a continuation of a large RPC\n");
		r535_gsp_msg_dump(gsp, rpc, NV_DBG_ERROR);
		return ERR_PTR(-EIO);
	}

Since they return without advancing *gsp->msgq.rptr, will subsequent attempts
to process the queue peek the exact same malformed message and hit the same
error, causing a permanently deadlocked queue?

> -		if (IS_ERR_OR_NULL(rpc)) {
> +		if (IS_ERR(rpc)) {
>  			kvfree(buf);
>  			return rpc;
>  		}
> @@ -359,7 +359,7 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
>  		info.continuation = true;
>  
>  		rpc = r535_gsp_msgq_recv_one_elem(gsp, &info);

[Severity: Critical]
This is a pre-existing issue, but does this code lack upper and lower bounds
checks on the untrusted rpc->length field from GSP messages?

If a compromised GSP provides a small length for a continuation record, it
looks like len - sizeof(struct nvfw_gsp_rpc) in r535_gsp_msgq_recv_one_elem()
could underflow:

	if (info->continuation)
		memcpy(buf, mqe->data + sizeof(struct nvfw_gsp_rpc),
		       len - sizeof(struct nvfw_gsp_rpc));

Additionally, a few lines below this hunk in r535_gsp_msgq_recv():

		size = info.gsp_rpc_len - sizeof(*rpc);
		expected -= size;
		info.gsp_rpc_buf += size;

Could this overflow the kvmalloc-allocated buffer since there is no upper
bound check ensuring rpc->length does not exceed expected?

> -		if (IS_ERR_OR_NULL(rpc)) {
> +		if (IS_ERR(rpc)) {
>  			kvfree(buf);
>  			return rpc;
>  		}

[ ... ]

> @@ -499,7 +499,7 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
>  
>  retry:
>  	rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), &retries);
> -	if (IS_ERR_OR_NULL(rpc))
> +	if (IS_ERR(rpc))
>  		return rpc;
>  
>  	rpc = r535_gsp_msgq_recv(gsp, gsp_rpc_len, &retries);

[Severity: High]
This is a pre-existing issue, but does r535_gsp_rpc_push() leak large GSP RPC
messages on error paths?

If the payload size exceeds max_payload_size, the message is chunked and the
original msg must be freed manually. However, the error paths seem to bypass
this:

r535_gsp_rpc_push() {
...
done:
	mutex_unlock(&gsp->cmdq.mutex);
	return repv;
}

If any intermediate chunk allocation or sending step fails and triggers the
goto done path, does this bypass the kvfree(msg) call and leak the large
message allocation?

[Severity: High]
This isn't a bug introduced by this patch, but does r535_gsp_msgq_wait()
miss a read memory barrier when polling the GSP message queue write pointer?

r535_gsp_msgq_wait() {
...
	if (WARN_ON(!*ptime))
		return -ETIMEDOUT;

	return used;
}

Since there is no smp_rmb() between checking the write pointer and reading
the payload from shared memory, could the CPU speculatively execute the
payload reads before the pointer read completes on weakly ordered architectures,
observing stale memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622081136.61695-1-zenghongling@kylinos.cn?part=2

Re: [PATCH v5 5/5] nouveau/gsp: Cleanup IS_ERR_OR_NULL in nvkm_gsp_rpc_rd()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Integer underflow in `r535_gsp_msgq_recv_one_elem` due to missing lower bounds check on `rpc->length` causes massive out-of-bounds memory copy.
- [Critical] A Time-of-Check to Time-of-Use (TOCTOU) double-fetch vulnerability in `r535_gsp_msgq_recv` allows a malicious GSP firmware to cause a kernel heap buffer overflow.
- [High] NULL pointer dereference when `r535_gsp_msgq_recv()` encounters an invalid RPC message size.
- [High] Out-of-bounds memory reads due to unvalidated array counts from firmware in `r535_gsp_intr_get_table` and `r535_gsp_get_static_info_fb`.
--

commit 88b24214ef8688e688f55f1aef5cd0f776cdce80
Author: Hongling Zeng <zenghongling@kylinos.cn>
Subject: nouveau/gsp: Cleanup IS_ERR_OR_NULL in nvkm_gsp_rpc_rd()

This commit cleans up the error checking in nvkm_gsp_rpc_rd(). Since
nvkm_gsp_rpc_get() returns only valid pointers or error pointers, the
code is updated to use IS_ERR() instead of IS_ERR_OR_NULL() to match the
actual API contract.

> diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
> index 64fed208e4cf8..a061779e0107b 100644
> --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
> +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
> @@ -293,7 +293,7 @@ nvkm_gsp_rpc_rd(struct nvkm_gsp *gsp, u32 fn, u32 argc)
>  {
>  	void *argv = nvkm_gsp_rpc_get(gsp, fn, argc);
>  
> -	if (IS_ERR_OR_NULL(argv))
> +	if (IS_ERR(argv))
>  		return argv;
>  
>  	return nvkm_gsp_rpc_push(gsp, argv, NVKM_GSP_RPC_REPLY_RECV, argc);

[Severity: High]
This isn't a bug introduced by this patch, but if r535_gsp_msgq_recv() 
returns NULL when rpc->length exceeds max_rpc_size, doesn't this bubble up 
through nvkm_gsp_rpc_push() and nvkm_gsp_rpc_rd()? 

If callers like r535_gsp_get_static_info() only check IS_ERR(), won't they 
dereference NULL? 

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c:r535_gsp_get_static_info() {
    ...
	rpc = nvkm_gsp_rpc_rd(gsp, NV_VGPU_MSG_FUNCTION_GET_GSP_STATIC_INFO, sizeof(*rpc));
	if (IS_ERR(rpc))
		return PTR_ERR(rpc);

	gsp->internal.client.object.client = &gsp->internal.client;
	...
	gsp->internal.client.object.handle = rpc->hInternalClient;
    ...
}

Should r535_gsp_msgq_recv() return an error pointer like ERR_PTR(-EIO) 
instead of NULL to match this API contract?

[Severity: High]
This is a pre-existing issue, but does r535_gsp_intr_get_table() properly 
bound its iteration over ctrl->tableLen? 

If a faulty firmware provides a count larger than 
NV2080_CTRL_INTERNAL_INTR_MAX_TABLE_SIZE, won't this loop read out-of-bounds 
into adjacent memory? 

Similarly, does r535_gsp_get_static_info_fb() need to verify 
info->numFBRegions against the fbRegion array bounds before iterating?

[Severity: Critical]
This isn't a bug introduced by this patch, but is there a missing lower 
bounds check for rpc->length in r535_gsp_msgq_recv()? 

If rpc->length is 0, len evaluates to 0 for continuation records in 
r535_gsp_msgq_recv_one_elem(). Doesn't len - sizeof(struct nvfw_gsp_rpc) 
then underflow, causing a massive out-of-bounds memory copy?

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c:r535_gsp_msgq_recv_one_elem() {
    ...
	if (info->continuation)
		memcpy(buf, mqe->data + sizeof(struct nvfw_gsp_rpc),
		       len - sizeof(struct nvfw_gsp_rpc));
    ...
}

[Severity: Critical]
This is a pre-existing issue, but does r535_gsp_msgq_recv() safely handle 
rpc->length from shared memory? 

It appears the firmware-provided rpc->length is read directly from mqe->data 
three times. If a malicious firmware increases this value after the bounds 
check and allocation, but before info.gsp_rpc_len = rpc->length, couldn't 
r535_gsp_msgq_recv_one_elem() cause a heap overflow when copying up to 
info.gsp_rpc_len bytes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622081136.61695-1-zenghongling@kylinos.cn?part=5

Re: [PATCH v5 1/5] nouveau/gsp/rpc: Document RPC function return value contracts

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing bounds check on a continuation record's length leads to massive integer underflow and a kernel panic.
- [High] Memory leak in `r535_gsp_rpc_push()` when an error occurs while handling a large payload that requires chunking.
- [High] The `r535_gsp_msgq_recv` function returns `NULL` on an invalid RPC length, which is incorrectly propagated as a success state.
- [High] Data corruption and uninitialized memory gaps occur when a large continuation record wraps around the message queue ring buffer.
--

commit 1bc0e8927764cf75b9678605ae80d0c1fd535eb5
Author: Hongling Zeng <zenghongling@kylinos.cn>
Subject: nouveau/gsp/rpc: Document RPC function return value contracts

This commit adds kernel-doc comments to document the return value semantics
of RPC functions in r535/rpc.c. It clarifies which functions return NULL,
error pointers, or both to help validate future changes.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
> index 3ca3de8f43408..b4b2b5c00ab4e 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c

[ ... ]

> @@ -229,6 +238,14 @@ struct r535_gsp_msg_info {
>  static void
>  r535_gsp_msg_dump(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc *msg, int lvl);
>  
> +/**
> + * r535_gsp_msgq_recv_one_elem - Receive one message element from GSP queue
> + * @gsp: GSP device
> + * @info: Message queue information
> + *
> + * Return: Pointer to received buffer on success, or ERR_PTR() on error.
> + *         Never returns NULL.
> + */
>  static void *
>  r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,

[Severity: Critical]
This is a pre-existing issue, but is there a missing bounds check on the
continuation record length here?

In r535_gsp_msgq_recv(), the length of a continuation record is read
directly from the hardware:

    info.gsp_rpc_len = rpc->length;
    info.continuation = true;

If a compromised or buggy GSP firmware returns a length smaller than
sizeof(struct nvfw_gsp_rpc) (32 bytes), len becomes less than 32 in
r535_gsp_msgq_recv_one_elem(). The subtraction:

    len - sizeof(struct nvfw_gsp_rpc)

will underflow to a massive positive value, causing memcpy() to attempt
an out-of-bounds copy and panic the kernel.

[Severity: High]
This is another pre-existing issue in this function, but does this code
cause data corruption when a continuation record wraps around the ring
buffer?

When copying a continuation record, the 32-byte header is stripped in the
first memcpy():

    memcpy(buf, mqe->data + sizeof(struct nvfw_gsp_rpc),
           len - sizeof(struct nvfw_gsp_rpc));

However, if the element wraps around, the second memcpy() writes to
buf + len:

    if (expected) {
            mqe = (void *)((u8 *)gsp->shm.msgq.ptr + 0x1000 + 0 * 0x1000);
            memcpy(buf + len, mqe, expected);
    }

Should this second memcpy() be writing to buf + len - sizeof(struct nvfw_gsp_rpc)
instead? Writing to buf + len leaves a 32-byte gap of uninitialized memory
and shifts the tail data forward, which will be overwritten by the next
continuation payload.

[ ... ]

> @@ -283,6 +300,15 @@ r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
>  	return buf;
>  }
>  
> +/**
> + * r535_gsp_msgq_recv - Receive RPC message(s) from GSP queue
> + * @gsp: GSP device
> + * @gsp_rpc_len: Expected RPC length
> + * @retries: Retry counter
> + *
> + * Return: Pointer to received buffer on success, ERR_PTR() on error,
> + *         or NULL if RPC length is invalid.
> + */

[Severity: High]
This isn't a bug introduced by this patch, but does the documentation here
cement a flawed design?

If rpc->length > max_rpc_size, r535_gsp_msgq_recv() returns NULL. This
propagates through r535_gsp_msg_recv() and r535_gsp_rpc_handle_reply().
Because NULL is also the expected success return value for POLL mode,
callers like r535_gsp_rpc_poll() treat the invalid length error as a
success. Other callers expecting ERR_PTR on error might dereference NULL.

>  static void *
>  r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)

[ ... ]

> @@ -628,6 +704,17 @@ r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 payload_size)
>  	return rpc->data;
>  }
>  
> +/**
> + * r535_gsp_rpc_push - Push RPC message to GSP and wait for reply
> + * @gsp: GSP device
> + * @payload: RPC payload to send
> + * @policy: Reply policy (NOWAIT, NOSEQ, RECV, or POLL)
> + * @gsp_rpc_len: Expected RPC length in the reply
> + *
> + * Return: NULL when policy is NOWAIT/NOSEQ (no reply expected) or
> + *         POLL (successful message processing), pointer to reply data
> + *         on success, or ERR_PTR() on error.
> + */
>  static void *
>  r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload,

[Severity: High]
This isn't a bug introduced by this patch, but does this function leak memory
on the error path for large payloads?

When payload_size > max_payload_size, the original msg buffer is not passed
to r535_gsp_cmdq_push(), which would normally free it. Instead, it is manually
freed at the end of the chunking process:

    if (!IS_ERR(repv))
            kvfree(msg);

If any step fails during the chunking process, such as r535_gsp_rpc_send()
returning an ERR_PTR(), the code branches to the done label:

    if (IS_ERR(repv))
            goto done;

This bypasses the kvfree(msg) check at the end of the block, permanently
leaking the msg allocation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622081136.61695-1-zenghongling@kylinos.cn?part=1

Re: [PATCH v5 3/5] nouveau/gsp/rm/alloc: Cleanup IS_ERR_OR_NULL usage

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Corrupt or oversized GSP messages cause NULL pointer dereferences or false successes because they return NULL instead of an error pointer.
- [High] Leaking the original large RPC payload message in `r535_gsp_rpc_push()` on error paths during chunked transmission.
--

commit be006c1667937e9a2375afa2299598f63205492b
Author: Hongling Zeng <zenghongling@kylinos.cn>
Subject: nouveau/gsp/rm/alloc: Cleanup IS_ERR_OR_NULL usage

This commit changes the error checking in r535_gsp_rpc_rm_free() to use
IS_ERR() instead of IS_ERR_OR_NULL() for the return value of
nvkm_gsp_rpc_get().

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
> index 46e3a29f2ad7f..c9f86c0e9b255 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
[ ... ]
> @@ -35,7 +35,7 @@ r535_gsp_rpc_rm_free(struct nvkm_gsp_object *object)
>  		   client->object.handle, object->handle);
>  
>  	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_FREE, sizeof(*rpc));
> -	if (WARN_ON(IS_ERR_OR_NULL(rpc)))
> +	if (WARN_ON(IS_ERR(rpc)))

[Severity: High]
This isn't a bug introduced by this patch, but while we are refining the use of
IS_ERR() for RPC messages, could corrupt or oversized GSP messages cause a NULL
pointer dereference or false success elsewhere?

Looking at r535_gsp_msgq_recv() in
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c, if an RPC message length
exceeds the maximum size, it issues a WARN_ON and returns NULL instead of an
ERR_PTR:

r535_gsp_msgq_recv() {
    ...
    if (WARN_ON(rpc->length > max_rpc_size))
        return NULL;
    ...
}

This NULL propagates up through r535_gsp_msg_recv() and nvkm_gsp_rpc_push()
back to wrappers. Callers expecting a payload use IS_ERR() to check for errors,
but since IS_ERR(NULL) is false, do these callers incorrectly treat the failure
as a success?

For example, in r535_gsp_get_static_info():

r535_gsp_get_static_info() {
    ...
    rpc = nvkm_gsp_rpc_rd(gsp, NV_VGPU_MSG_FUNCTION_GET_GSP_STATIC_INFO, sizeof(*rpc));
    if (IS_ERR(rpc))
        return PTR_ERR(rpc);

    gsp->internal.client.object.client = &gsp->internal.client;
    gsp->internal.client.object.parent = NULL;
    gsp->internal.client.object.handle = rpc->hInternalClient;
    ...
}

Could this bypass lead to a direct NULL pointer dereference, or cause silent
hardware operation failures in functions like nvkm_gsp_rpc_wr() and
nvkm_gsp_rm_alloc_wr() that similarly only check IS_ERR()?

[Severity: High]
This is also a pre-existing issue, but while reviewing the RPC push logic, is
there a memory leak in r535_gsp_rpc_push() for large payloads on error paths?

When the payload size exceeds the maximum, r535_gsp_rpc_push() splits it into
continuation chunks. The original message memory, allocated by
nvkm_gsp_rpc_get(), is not passed to the command queue (which would normally
take ownership). Instead, r535_gsp_rpc_push() attempts to free it manually
only upon successful reply handling:

r535_gsp_rpc_push() {
    ...
        /* Wait for reply. */
        repv = r535_gsp_rpc_handle_reply(gsp, fn, policy, payload_size +
                                         sizeof(*rpc));
        if (!IS_ERR(repv))
            kvfree(msg);
    } else {
        repv = r535_gsp_rpc_send(gsp, payload, policy, gsp_rpc_len);
    }

done:
    mutex_unlock(&gsp->cmdq.mutex);
    return repv;
}

If sending the chunks fails or if the reply contains an error (such as the GSP
rejecting the command), the function jumps to the done label and returns
without freeing the message.

Since callers typically overwrite their pointer with the return value of
nvkm_gsp_rpc_push() and assume ownership was passed, does this permanently
leak the original payload memory if userspace issues large RPCs that get
rejected?

>  		return -EIO;
>  
>  	rpc->params.hRoot = client->object.handle;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622081136.61695-1-zenghongling@kylinos.cn?part=3

Re: [PATCH v5 2/5] nouveau/gsp/rpc: Cleanup incorrect IS_ERR_OR_NULL in rpc.c

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The driver blindly trusts the `length` field of GSP messages and continuation records, leading to multiple integer underflows and massive out-of-bounds memory copies.
- [Critical] Missing bounds check on GSP continuation message length allows a heap buffer overflow.
- [High] Returning `NULL` on oversized GSP messages leads to infinite loops in the message queue worker and a false success bubbling up to callers, causing subsequent NULL pointer dereferences.
- [High] A continuation message wrapping around the message queue ring buffer causes memory corruption and an overrun due to incorrect pointer math.
- [High] Missing read memory barrier (rmb or dma_rmb) after reading the ring buffer write pointer.
--

commit b655637a9811533563a43c05f87d130fe957a519
Author: Hongling Zeng <zenghongling@kylinos.cn>
Subject: nouveau/gsp/rpc: Cleanup incorrect IS_ERR_OR_NULL in rpc.c

This commit cleans up IS_ERR_OR_NULL() usage for functions that supposedly
never return NULL. It changes the checks after r535_gsp_msgq_peek(),
r535_gsp_msgq_recv_one_elem(), and r535_gsp_msgq_recv() to use IS_ERR()
instead.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
> index b4b2b5c00ab4e..5b89c8835a2e4 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
> @@ -350,7 +350,7 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
>  		u32 size;
>  
>  		rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), info.retries);
> -		if (IS_ERR_OR_NULL(rpc)) {
> +		if (IS_ERR(rpc)) {

[Severity: High]
This is a pre-existing issue, but does the polling loop in
r535_gsp_msgq_wait() need a read memory barrier?

r535_gsp_msgq_peek() calls r535_gsp_msgq_wait(), which polls the GSP write
pointer *gsp->msgq.wptr from DMA coherent memory:

    while (rptr == wptr || ...) {
        ...
        wptr = *gsp->msgq.wptr;
    }
    
Without an rmb() or dma_rmb() before returning, could weakly ordered CPUs
speculatively fetch the payload data before the write pointer read is
ordered, resulting in stale data being read from the ring buffer?

>  			kvfree(buf);
>  			return rpc;
>  		}
> @@ -359,7 +359,7 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
>  		info.continuation = true;
>  
>  		rpc = r535_gsp_msgq_recv_one_elem(gsp, &info);
> -		if (IS_ERR_OR_NULL(rpc)) {
> +		if (IS_ERR(rpc)) {

[Severity: Critical]
This isn't a bug introduced by this patch, but does the firmware's
message length need to be validated to prevent integer underflows?

If rpc->length is smaller than sizeof(struct nvfw_gsp_rpc), does
this underflow the length calculations and cause an out-of-bounds memcpy()?

In r535_gsp_msgq_recv_one_elem(), the payload length calculation uses:

    if (info->continuation)
        memcpy(buf, mqe->data + sizeof(struct nvfw_gsp_rpc),
               len - sizeof(struct nvfw_gsp_rpc));
               
Additionally, in this loop in r535_gsp_msgq_recv(), the offset underflows:

    size = info.gsp_rpc_len - sizeof(*rpc);
    expected -= size;
    info.gsp_rpc_buf += size;
    
There is a similar underflow in r535_gsp_msg_dump():

    msg->length - sizeof(*msg)
    
Could a malicious or buggy firmware send a small length and cause these
calculations to wrap around, leading to massive memory corruption?

[Severity: Critical]
This is a pre-existing issue, but does this code check if the continuation
message actually fits within the remaining allocated buffer?

The buffer was allocated based on the initial expected message length.
If a continuation chunk payload (info.gsp_rpc_len - sizeof(*rpc)) is
larger than the remaining capacity (expected), does this overwrite the
bounds of the kvmalloc'd buffer?

[Severity: High]
This isn't a bug introduced by this patch, but is there a miscalculation
when a continuation message wraps around the ring buffer?

In r535_gsp_msgq_recv_one_elem(), the code copies a wrapped continuation
payload in two parts. The first part writes len - sizeof(struct nvfw_gsp_rpc)
bytes. However, for the second part it copies:

    if (expected) {
        mqe = (void *)((u8 *)gsp->shm.msgq.ptr + 0x1000 + 0 * 0x1000);
        memcpy(buf + len, mqe, expected);
    }

Should the destination pointer be buf + len - sizeof(struct nvfw_gsp_rpc)
instead of buf + len? Does using buf + len leave a gap of uninitialized
memory and subsequently overrun the allocated buffer by
sizeof(struct nvfw_gsp_rpc) bytes?

>  			kvfree(buf);
>  			return rpc;
>  		}
> @@ -499,7 +499,7 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
>  
>  retry:
>  	rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), &retries);
> -	if (IS_ERR_OR_NULL(rpc))
> +	if (IS_ERR(rpc))
>  		return rpc;
>  
>  	rpc = r535_gsp_msgq_recv(gsp, gsp_rpc_len, &retries);

[Severity: High]
This is a pre-existing issue, but is it safe for r535_gsp_msgq_recv() to
return NULL when a message is oversized?

r535_gsp_msgq_recv() has the following check:

    if (WARN_ON(rpc->length > max_rpc_size))
        return NULL;
        
If an oversized message is received, this returns NULL instead of advancing
the message queue read pointer and returning an error pointer like
ERR_PTR(-EIO).

Does this cause the message queue worker to get stuck in an infinite loop
repeatedly processing the same invalid message? Also, could this NULL bubble
up as a false success to callers that check for errors via IS_ERR(), causing
subsequent NULL pointer dereferences?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622081136.61695-1-zenghongling@kylinos.cn?part=2

Re: [PATCH v5 4/5] nouveau/gsp/rm/bar: Cleanup IS_ERR_OR_NULL usage

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] `r535_gsp_msgq_recv()` incorrectly returns `NULL` instead of an `ERR_PTR` when receiving a message exceeding `max_rpc_size`, leading to NULL pointer dereferences in upstream callers.
- [Medium] In `r535_gsp_rpc_push()`, when a large RPC payload encounters an error during transmission or waiting for a reply, the original message buffer is leaked.
--

commit c28b7cbd3dab1fd32a31ef4df86d6f6aaf9ad40b
Author: Hongling Zeng <zenghongling@kylinos.cn>

nouveau/gsp/rm/bar: Cleanup IS_ERR_OR_NULL usage

This commit updates r535_bar_bar2_update_pde() to use IS_ERR() instead of
IS_ERR_OR_NULL() when checking the return value of nvkm_gsp_rpc_get(). This
aligns with the function's contract of never returning NULL.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
> index fae08ac3b18c7..9cd68f8622d36 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/bar.c
> @@ -55,7 +55,7 @@ r535_bar_bar2_update_pde(struct nvkm_gsp *gsp, u8 page_shift, u64 pdbe)
>  	rpc_update_bar_pde_v15_00 *rpc;
>  
>  	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_UPDATE_BAR_PDE, sizeof(*rpc));
> -	if (WARN_ON(IS_ERR_OR_NULL(rpc)))
> +	if (WARN_ON(IS_ERR(rpc)))
>  		return -EIO;
>  
>  	rpc->info.barType = NV_RPC_UPDATE_PDE_BAR_2;

[Severity: High]
This is a pre-existing issue, but there is a case where the GSP RPC
infrastructure incorrectly returns a NULL pointer. If the message queue
receives an unexpectedly large message:

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c:r535_gsp_msgq_recv() {
	mqe = r535_gsp_msgq_get_entry(gsp);
	rpc = (struct nvfw_gsp_rpc *)mqe->data;

	if (WARN_ON(rpc->length > max_rpc_size))
		return NULL;
...
}

This NULL return value propagates up through r535_gsp_msg_recv() and
r535_gsp_rpc_handle_reply(), causing nvkm_gsp_rpc_rd() to return NULL.

Does this lead to a NULL pointer dereference in callers like
r535_gsp_get_static_info() since they only validate the result with IS_ERR()?

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c:r535_gsp_get_static_info() {
	rpc = nvkm_gsp_rpc_rd(gsp, NV_VGPU_MSG_FUNCTION_GET_GSP_STATIC_INFO, sizeof(*rpc));
	if (IS_ERR(rpc))
		return PTR_ERR(rpc);

	gsp->internal.client.object.client = &gsp->internal.client;
...
	gsp->internal.client.object.handle = rpc->hInternalClient;
...
}

[Severity: Medium]
This is also a pre-existing issue, but does the slow path in
r535_gsp_rpc_push() leak the original message buffer when an error occurs?

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c:r535_gsp_rpc_push() {
...
	if (payload_size > max_payload_size) {
...
		/* Send initial RPC. */
		next = r535_gsp_rpc_get(gsp, fn, max_payload_size);
		if (IS_ERR(next)) {
			repv = next;
			goto done;
		}
...
		/* Wait for reply. */
		repv = r535_gsp_rpc_handle_reply(gsp, fn, policy, payload_size +
						 sizeof(*rpc));
		if (!IS_ERR(repv))
			kvfree(msg);
	} else {
...
done:
	mutex_unlock(&gsp->cmdq.mutex);
	return repv;
}

In the fast path, r535_gsp_cmdq_push() takes ownership and unconditionally
frees the buffer. However, in the chunked sending path above, if there is an
error getting a chunk (IS_ERR(next)) or receiving a reply (IS_ERR(repv)),
execution jumps to the done label without calling kvfree(msg). Could this
permanently leak the large payload buffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622081136.61695-1-zenghongling@kylinos.cn?part=4

Re: [PATCH v5 5/5] nouveau/gsp: Cleanup IS_ERR_OR_NULL in nvkm_gsp_rpc_rd()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] NULL pointer dereference when GSP firmware sends a malformed message with invalid length.
- [Medium] Memory leak of large RPC request buffers on error paths.
--

commit 71d7ed95d2b3f836cf2bbe162c7b67d89c46964e
Author: Hongling Zeng <zenghongling@kylinos.cn>

nouveau/gsp: Cleanup IS_ERR_OR_NULL in nvkm_gsp_rpc_rd()

This patch changes nvkm_gsp_rpc_rd() to check IS_ERR() instead of
IS_ERR_OR_NULL() for the return value of nvkm_gsp_rpc_get(). It also
notes that nvkm_gsp_rpc_push() should return error pointers, not NULL,
when using the RECV policy.

> Note: nvkm_gsp_rpc_push() can return NULL when policy is NOWAIT or
> NOSEQ, but nvkm_gsp_rpc_rd() uses RECV policy which expects a reply.
> If the RPC fails to get a reply, it should return an error pointer,
> not NULL.
>
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

[Severity: High]
This is a pre-existing issue, but is the assumption in the commit message
accurate?

Looking at r535_gsp_msgq_recv() in rpc.c, if a malformed message is
received, it explicitly returns NULL:

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c:r535_gsp_msgq_recv() {
	if (WARN_ON(rpc->length > max_rpc_size))
		return NULL;
	...
}

This NULL propagates through the reply handlers to nvkm_gsp_rpc_push(),
which returns it to the caller.

Because callers like r535_gsp_get_static_info() only check IS_ERR(),
this leads to a NULL pointer dereference:

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c:r535_gsp_get_static_info() {
	if (IS_ERR(rpc))
		return PTR_ERR(rpc);

	gsp->internal.client.object.client = &gsp->internal.client;
	...
}

Other callers like r535_gsp_rpc_rm_ctrl_push() implicitly convert this NULL
return to 0 (success) via PTR_ERR(rpc), masking the failure.

Should the underlying r535_gsp_msgq_recv() be modified to return an
ERR_PTR to match the expected contract?

> diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
> index 64fed208e4cf8..a061779e0107b 100644
> --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
> +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
> @@ -293,7 +293,7 @@ nvkm_gsp_rpc_rd(struct nvkm_gsp *gsp, u32 fn, u32 argc)
>  {
>  	void *argv = nvkm_gsp_rpc_get(gsp, fn, argc);
>  
> -	if (IS_ERR_OR_NULL(argv))
> +	if (IS_ERR(argv))
>  		return argv;
>  
>  	return nvkm_gsp_rpc_push(gsp, argv, NVKM_GSP_RPC_REPLY_RECV, argc);
        ^^^^

[Severity: Medium]
This is a pre-existing issue, but does r535_gsp_rpc_push() leak the msg
buffer on error paths?

Looking at r535_gsp_rpc_push() in rpc.c, when payload_size > max_payload_size,
the large msg buffer is freed only on success:

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c:r535_gsp_rpc_push() {
	...
done:
	if (!IS_ERR(repv))
		kvfree(msg);

	return repv;
}

If any intermediate chunk allocation fails, or if the final reply results
in an error pointer, could this leak the large RPC request buffers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622081136.61695-1-zenghongling@kylinos.cn?part=5