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

[PATCH v4 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 v4 1/5] 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, 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 v4:
 -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..ae1820e6a9e7 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_get() 
+ */
 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 v4 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 v4:
 -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..ae1820e6a9e7 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_get() 
+ */
 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 v4 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 v4:
 -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..ae1820e6a9e7 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_get() 
+ */
 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 v4 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 ae1820e6a9e7..3e2f27309b00 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 v4 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 ae1820e6a9e7..3e2f27309b00 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 v4 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 ae1820e6a9e7..3e2f27309b00 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 v4 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 v4 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 v4 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 v4 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 v4 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 v4 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 v4 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 v4 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] Large GSP RPC messages are leaked when an error occurs during chunked transmission.
--

commit fa8eea5c56bcb72beec86aeaab70f0ec3eed80b9
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;
>  
>  	rpc->info.barType = NV_RPC_UPDATE_PDE_BAR_2;

[Severity: High]
This is a pre-existing issue, but while looking at the GSP RPC handling, I
noticed a potential memory leak in r535_gsp_rpc_push().

When transmitting large GSP RPC payloads (where payload_size is larger than
max_payload_size), the original large message buffer (msg) can be leaked if
an error occurs.

If an error occurs during the chunking process, such as failing to allocate
a smaller chunk, the code jumps to the done label:

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c:r535_gsp_rpc_push() {
    ...
		next = r535_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD, size);
		if (IS_ERR(next)) {
			repv = next;
			goto done;
		}
    ...

The same happens if there's a timeout or error from r535_gsp_rpc_send():

			repv = r535_gsp_rpc_send(gsp, next, NVKM_GSP_RPC_REPLY_NOWAIT, 0);
			if (IS_ERR(repv))
				goto done;
    ...

Also, if r535_gsp_rpc_handle_reply() returns an error, the code skips the free:

		/* 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;
}

Since the done label only drops the mutex and returns, doesn't this
permanently leak the msg buffer on any of these error paths?

Could this exacerbate memory pressure or lead to out of memory conditions if
large RPC transmissions repeatedly fail?

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

Re: [PATCH v4 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 7 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing validation of GSP RPC message lengths from firmware can lead to integer underflow and massive kernel memory corruption.
- [High] Silent failure when rpc is NULL results in caller NULL pointer dereference.
- [High] Buffer overflow and uninitialized memory gap when a continuation GSP RPC chunk wraps around the message queue ring buffer.
- [High] Silent infinite stall of the GSP message queue when an oversized message is encountered.
- [High] Missing read memory barrier (`rmb()`) between reading the shared memory write pointer and reading the message payload.
- [High] Time-of-Check to Time-of-Use race condition on `rpc->length` bypasses bounds checks.
- [High] Trusting the CPU read pointer (`rptr`) from shared memory leads to arbitrary memory access.
--

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

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

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.

> 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 2ca6de4094e63..0a00c7d831c91 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)

[Severity: High]
This is a pre-existing issue, but earlier in r535_gsp_msgq_recv(),
returning NULL when an oversized message is encountered seems to stall the
queue:

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

Does returning NULL here without advancing the queue read pointer cause an
unrecoverable infinite stall, since subsequent reads would encounter the
exact same oversized message?

[Severity: High]
This isn't a bug introduced by this patch, but there also appears to be a
Time-of-Check to Time-of-Use race condition on rpc->length:

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

    buf = kvmalloc(max_t(u32, rpc->length, expected), GFP_KERNEL);
    ...
}

Since rpc points directly to DMA coherent memory shared with the GSP, could
the length be overwritten in shared memory between the check and its use in
kvmalloc() or info.gsp_rpc_len, bypassing the validation?

>  		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);

[Severity: High]
This is a pre-existing issue, but in r535_gsp_msgq_recv_one_elem() called
here, is there a potential integer underflow?

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

If the continuation message length read directly from shared memory is less
than sizeof(struct nvfw_gsp_rpc) (32 bytes), does this calculation underflow
and lead to a massive out-of-bounds read and write?

[Severity: High]
This is another pre-existing issue in r535_gsp_msgq_recv_one_elem(). Could
a continuation chunk wrapping around the ring buffer cause an out-of-bounds
write?

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

If the first copy only advanced the buffer logically by len - 32 due to
skipping the header, would writing the second part at buf + len leave a
32-byte gap of uninitialized memory and overflow the end of the allocated
buffer by 32 bytes?

[Severity: High]
This isn't a bug introduced by this patch, but does the underlying
r535_gsp_msgq_wait() function lack a required memory barrier?

r535_gsp_msgq_wait() {
    ...
    u32 wptr = *gsp->msgq.wptr;
    ...
}

When polling the shared memory write pointer, does the host CPU need an rmb()
after observing the updated write pointer?

Without it, could the CPU speculatively fetch stale payload data before the
pointer update is fully observed?

[Severity: High]
This is a pre-existing issue, but does r535_gsp_msgq_get_entry() safely
handle the CPU read pointer?

r535_gsp_msgq_get_entry() {
    u32 rptr = *gsp->msgq.rptr;
    return (void *)((u8 *)gsp->shm.msgq.ptr + GSP_PAGE_SIZE +
                    rptr * GSP_PAGE_SIZE);
}

Since gsp->msgq.rptr points to DMA coherent memory, could a compromised GSP
modify this pointer to an arbitrarily large value and force the host to read
out-of-bounds host memory?

> -		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 isn't a bug introduced by this patch, but when r535_gsp_msgq_recv()
returns NULL (for example, when encountering an oversized message), does
this result in a silent failure upstream?

The NULL propagates out to r535_gsp_rpc_rm_ctrl_push(), which handles it
like this:

r535_gsp_rpc_rm_ctrl_push() {
    ...
    if (IS_ERR_OR_NULL(rpc)) {
        *params = NULL;
        return PTR_ERR(rpc);
    }
    ...
}

Since PTR_ERR(NULL) is 0 (success), does this mask the failure and cause
upstream callers to dereference a NULL pointer thinking the call succeeded?

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

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

[sashiko-bot]

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

New issues:
- [Medium] The documentation for r535_gsp_rpc_done() incorrectly states that it frees a payload returned by r535_gsp_rpc_get().

Pre-existing issues:
- [High] The newly added documentation for RPC functions introduces contradictory return value contracts that mask a pre-existing NULL pointer dereference bug on error paths. Specifically, `r535_gsp_msgq_recv()` returns `NULL` when the RPC length is invalid, which propagates up and causes a NULL pointer dereference in higher-level callers, contradicting the newly added docs that claim `ERR_PTR()` is returned on error.
- [High] Large RPC payloads (`payload_size > max_payload_size`) leak the original `msg` memory if an error occurs during chunk sending or reply handling.
--

commit ebdd2cc2dbf735e60a62853951f275c61160e93f
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 can return NULL or
error pointers.

> 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..2ca6de4094e63 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

[ ... ]

> @@ -663,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_get()
> + */
>  static void
>  r535_gsp_rpc_done(struct nvkm_gsp *gsp, void *repv)

[Severity: Medium]
Is this parameter description accurate?

The payload returned by r535_gsp_rpc_get() is the request, which is
automatically freed upon transmission in functions like r535_gsp_cmdq_push().

It appears r535_gsp_rpc_done() is intended to be used by callers to free
the reply payload returned by r535_gsp_rpc_push() or nvkm_gsp_rm_ctrl_rd().

Could documenting this as freeing the request payload encourage
callers to explicitly free it after sending, leading to a double-free?

[ ... ]

> @@ -704,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)

[Severity: High]
This is a pre-existing issue, but does this documentation mask an underlying
NULL pointer dereference?

r535_gsp_msgq_recv() can return NULL when the RPC length is invalid:

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

This NULL propagates up to r535_gsp_rpc_push() which the documentation
states will return ERR_PTR() on error. However, if r535_gsp_rpc_push()
returns NULL on an error path, callers like r535_gsp_rpc_rm_ctrl_push()
evaluate PTR_ERR(NULL) as 0 (success):

r535_gsp_rpc_rm_ctrl_push() {
    if (IS_ERR_OR_NULL(rpc)) {
        *params = NULL;
        return PTR_ERR(rpc);
    }
}

This causes callers like r535_disp_get_static_info() to assume success
and dereference the NULL pointer:

r535_disp_get_static_info() {
    ctrl = nvkm_gsp_rm_ctrl_rd(...);
    if (IS_ERR(ctrl))
        return PTR_ERR(ctrl);
    disp->wndw.mask = ctrl->windowPresentMask;
}

Should the documentation note this, or should the underlying NULL
propagation be fixed to return an error pointer?

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the
documentation for this function, it appears there is a memory leak
for large RPC payloads in r535_gsp_rpc_push().

When payload_size is greater than max_payload_size, the code allocates
smaller chunks and sends them sequentially. If any chunk fails to send,
the code takes the goto done path:

r535_gsp_rpc_push() {
    ...
    next = r535_gsp_rpc_get(gsp, fn, max_payload_size);
    if (IS_ERR(next)) {
        repv = next;
        goto done;
    }
    ...
}

At the end of the chunking block, the original msg is only freed if
repv is not an error pointer:

    repv = r535_gsp_rpc_handle_reply(...);
    if (!IS_ERR(repv))
        kvfree(msg);

Since callers expect the function to consume the payload and do not free
it on error, doesn't this permanently leak the original msg memory on
these error paths?

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

Re: [PATCH v4 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 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The GSP message queue receiver lacks bounds checking on continuation records, allowing a compromised GSP to overflow the host kernel heap.
- [High] `nvkm_gsp_rpc_push()` and `nvkm_gsp_rpc_rd()` can return `NULL` when expecting a reply (`RECV` policy), contrary to the commit message's claims, leading to NULL pointer dereferences in callers.
- [High] Memory leak of the original RPC payload (`msg`) in `r535_gsp_rpc_push()` when handling a large payload (`payload_size > max_payload_size`) and an error occurs.
--

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

nouveau/gsp: Cleanup IS_ERR_OR_NULL in nvkm_gsp_rpc_rd()

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.

[Severity: High]
This is a pre-existing issue, but the commit message states that
nvkm_gsp_rpc_push() cannot return NULL when using the RECV policy. Is this
accurate?

Looking at the call chain, it appears NULL can be returned and propagate up.
For example, r535_gsp_msgq_recv() returns NULL when the payload length
exceeds max bounds:

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

This NULL propagates through r535_gsp_msg_recv() to
r535_gsp_rpc_handle_reply() and then to nvkm_gsp_rpc_push() before being
returned by nvkm_gsp_rpc_rd().

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

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.handle = rpc->hInternalClient;
    ...
}

> 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 does nvkm_gsp_rpc_push() leak
memory on error paths for large payloads?

When processing multi-part chunks in r535_gsp_rpc_push(), the original
message buffer is conditionally freed only on success:

    if (IS_ERR(next)) {
        repv = next;
        goto done;
    }
...
    if (!IS_ERR(repv))
        kvfree(msg);
...
done:
    mutex_unlock(&gsp->cmdq.mutex);
    return repv;

If an error occurs, the function jumps to the done label and returns an
error pointer without freeing the msg buffer. Since nvkm_gsp_rpc_rd() expects
the push operation to consume and free the payload buffer, does this result
in a persistent memory leak?

[Severity: Critical]
This is a pre-existing issue, but does the GSP message queue receiver have a
bounds checking issue that could overflow the host kernel heap?

In r535_gsp_msgq_recv(), if the first message's length is larger than the
expected size, the subtraction underflows:

    info.gsp_rpc_buf += info.gsp_rpc_len;
    expected -= info.gsp_rpc_len;

The loop then continuously processes continuation records because expected is
now a large non-zero value. It peeks at the next message length and calls
r535_gsp_msgq_recv_one_elem(), which uses memcpy to blindly copy the data:

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

Could a compromised GSP firmware exploit this to overwrite adjacent memory?

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

Re: [PATCH v4 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] Heap buffer overflow due to integer underflow when parsing GSP RPC messages.
--

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

nouveau/gsp/rm/alloc: Cleanup IS_ERR_OR_NULL usage

This commit cleans up r535_gsp_rpc_rm_free() by replacing IS_ERR_OR_NULL()
with IS_ERR(). This matches the behavior of nvkm_gsp_rpc_get() which only
returns valid pointers or error pointers, never NULL.

> 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

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the RPC
parsing code, could the GSP message queue handling be vulnerable to an
integer underflow?

In r535_gsp_msgq_recv() in rpc.c, the firmware-provided rpc->length is read
from the untrusted GSP shared memory message queue:

r535_gsp_msgq_recv() {
        ...
        rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), info.retries);
        if (IS_ERR(rpc)) {
                kvfree(buf);
                return rpc;
        }

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

If this length is not validated to be at least sizeof(struct nvfw_gsp_rpc),
it appears we could underflow the length calculation in
r535_gsp_msgq_recv_one_elem() when processing continuation messages:

r535_gsp_msgq_recv_one_elem() {
        ...
        len = min_t(u32, expected, len);

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

If rpc->length (and therefore len) is less than sizeof(struct nvfw_gsp_rpc),
does len - sizeof(struct nvfw_gsp_rpc) underflow to a massive positive value,
leading to an out-of-bounds read and a massive heap buffer overflow into
buf during the memcpy?

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

Re: [PATCH v4 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 1 potential issue(s) to consider:

Pre-existing issues:
- [High] NULL pointer dereference and error masking when GSP firmware sends a malformed RPC message.
--

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

nouveau/gsp: Cleanup IS_ERR_OR_NULL in nvkm_gsp_rpc_rd()
    
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.

[Severity: High]
This isn't a bug introduced by this patch, but the commit message asserts
that nvkm_gsp_rpc_push() will return an error pointer instead of NULL under
the RECV policy. Is this assertion accurate?

If the GSP firmware sends a malformed message with an invalid size, the
receive path can return NULL even for RECV policies:

r535_gsp_msgq_recv() {
    ...
    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 nvkm_gsp_rpc_push().

> 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);

Since this function returns the result of nvkm_gsp_rpc_push() directly, it
can return NULL.

Since callers of nvkm_gsp_rpc_rd() typically only check IS_ERR(rpc), wouldn't
this result in a NULL pointer dereference?

For example:

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.handle = rpc->hInternalClient;
}

Furthermore, in r535_gsp_rpc_rm_ctrl_push(), a NULL return triggers an
IS_ERR_OR_NULL check but incorrectly translates to a 0 (success) return via
PTR_ERR(rpc), masking the error completely.

Should r535_gsp_msgq_recv() be updated to return an ERR_PTR(-EIO) instead of
NULL when a malformed RPC size is encountered?

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

Re: [PATCH v4 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 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Returning NULL on oversized RPC messages causes a permanent queue stall and a NULL pointer dereference in callers.
- [High] Missing lower-bound validation on `rpc->length` causes massive integer underflows, leading to out-of-bounds memory accesses and kernel heap corruption.
- [High] Incorrect destination offset when copying wrapped continuation payloads causes a 32-byte gap of uninitialized memory and a buffer overwrite.
- [High] Time-Of-Check to Time-Of-Use (TOCTOU) race when reading `rpc->length` from shared memory allows kernel heap buffer overflow.
--

commit 91659e1846d19fb0409c6de8d1d0817e283454ef
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, replacing them 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 2ca6de4094e63..0a00c7d831c91 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 isn't a bug introduced by this patch, but since you are cleaning up
error checking in r535_gsp_msgq_recv(), what happens when an incoming
message's length exceeds max_rpc_size?

Earlier in r535_gsp_msgq_recv(), the code does this:

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

Returning NULL here skips advancing the queue's read pointer (*gsp->msgq.rptr),
which seems like it could permanently stall the message queue as subsequent
reads will hit the exact same malformed message.

Additionally, this NULL return propagates back to callers like
r535_gsp_get_static_info(), which only check IS_ERR(rpc):

    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;

Will this cause a NULL pointer dereference when accessing rpc->hInternalClient?

[Severity: High]
This is a pre-existing issue, but could the direct read of rpc->length from
shared memory lead to a Time-Of-Check to Time-Of-Use (TOCTOU) race?

In r535_gsp_msgq_recv(), rpc->length is read and validated to determine the
allocation size:

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

    buf = kvmalloc(max_t(u32, rpc->length, expected), GFP_KERNEL);

However, it is read again from shared memory shortly after without READ_ONCE()
or caching the previously validated value:

    info.gsp_rpc_len = rpc->length;

If the value in shared memory is modified after the validation but before the
second read, could this bypass the size validation and result in a massive
out-of-bounds memcpy() into an undersized heap 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);

[Severity: High]
This is a pre-existing issue, but does r535_gsp_msgq_recv_one_elem() handle
wrapped continuation payloads correctly?

When a continuation message wraps around the end of the ring buffer, the code
skips the 32-byte RPC header for the first part:

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

However, for the wrapped part, it uses buf + len:

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

Since the first memcpy skipped 32 bytes of header, wouldn't using buf + len
leave a 32-byte gap of uninitialized memory in the destination buffer, and
subsequently overwrite past the end of the allocated buffer by 32 bytes?

> -		if (IS_ERR_OR_NULL(rpc)) {
> +		if (IS_ERR(rpc)) {

[Severity: High]
This isn't a bug introduced by this patch, but does the lack of a minimum
size check on rpc->length allow an integer underflow?

If the GSP provides an rpc->length that is smaller than the RPC header size
(sizeof(struct nvfw_gsp_rpc) which is 32 bytes), wouldn't the subtraction
in r535_gsp_msgq_recv_one_elem():

    len - sizeof(struct nvfw_gsp_rpc)

underflow to a massive unsigned integer, passing an enormous size to memcpy()
and causing an out-of-bounds read?

>  			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 there a similar underflow risk here in
r535_gsp_msg_recv() when calling notification callbacks?

Later in r535_gsp_msg_recv(), the length is subtracted again:

    if (ntfy->func)
        ntfy->func(ntfy->priv, ntfy->fn, rpc->data,
                   rpc->length - sizeof(*rpc));

Without a lower-bound validation on rpc->length, could this also underflow
and pass a massive size to the notification callbacks?

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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Returning NULL on an invalid RPC message length deadlocks the message queue and causes downstream wrappers to treat errors as success, leading to NULL pointer dereferences.
- [High] Lack of bounds checking on continuation chunks allows a malformed or malicious GSP message to cause integer underflows and heap buffer overflows.
- [High] The original split RPC message buffer is leaked on error paths when the payload exceeds max_payload_size.
--

commit b2d404f83fcf801450f3eade502dc33e20d3fbe6
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 the Nouveau GSP RM r535/rpc.c file.

> 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..2ca6de4094e63 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.
> + */
>  static void *
>  r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
>  {

[Severity: High]
This is a pre-existing issue, but does returning NULL here when the RPC
length is invalid cause the message queue to deadlock and trigger NULL
pointer dereferences downstream?

Looking at r535_gsp_msgq_recv(), it returns NULL without advancing the read
pointer:

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

Several downstream functions seem to mishandle this NULL return. For
example, in r535_gsp_rpc_rm_ctrl_push():

	if (IS_ERR_OR_NULL(rpc)) {
		*params = NULL;
		return PTR_ERR(rpc);
	}

This converts IS_ERR_OR_NULL(NULL) to PTR_ERR(NULL) == 0, signaling
success to the caller while setting *params to NULL.

Similarly, nvkm_gsp_rm_alloc_wr() checks IS_ERR(repv) and returns success
since IS_ERR is false for NULL. Callers like r535_gsp_get_static_info()
then check the reply with IS_ERR() instead of IS_ERR_OR_NULL(), leading
directly to a NULL pointer dereference.


[Severity: High]
This is also a pre-existing issue in r535_gsp_msgq_recv(), but can a
malformed GSP message cause integer underflows and heap overflows during
continuation chunk processing?

In the continuation loop:

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

	rpc = r535_gsp_msgq_recv_one_elem(gsp, &info);
	...
	size = info.gsp_rpc_len - sizeof(*rpc);
	expected -= size;

If rpc->length is less than sizeof(*rpc), the subtraction for size will
underflow to a massive value, which then appears to cause an out-of-bounds
memcpy in r535_gsp_msgq_recv_one_elem(). Is there a missing bounds check
against the remaining expected capacity and the minimum header 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)

[Severity: High]
This is a pre-existing issue, but does this function leak the original
split RPC message buffer on error paths?

In r535_gsp_rpc_push(), when the payload exceeds max_payload_size, it is
chunked and sent iteratively. If an intermediate chunk fails to send, the
code jumps to the done label:

			repv = r535_gsp_rpc_send(gsp, next, NVKM_GSP_RPC_REPLY_NOWAIT, 0);
			if (IS_ERR(repv))
				goto done;
...
		if (!IS_ERR(repv))
			kvfree(msg);

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

Because kvfree(msg) is only executed if the operations are successful
(!IS_ERR(repv)), does the original msg buffer remain permanently leaked
when an error occurs?

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

Re: [PATCH v4 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] Memory leak of the RPC message buffer in r535_gsp_rpc_push() when an error occurs while sending large payloads.
- [High] Malformed GSP firmware messages cause a NULL pointer dereference via nvkm_gsp_rm_ctrl_rd().
--

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

nouveau/gsp/rm/bar: Cleanup IS_ERR_OR_NULL usage

This commit cleans up the usage of IS_ERR_OR_NULL in
r535_bar_bar2_update_pde() by using IS_ERR() instead, matching the
actual return value contract of nvkm_gsp_rpc_get() which never returns
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 isn't a bug introduced by this patch, but while reviewing this cleanup,
I noticed another IS_ERR_OR_NULL usage in r535_gsp_rpc_rm_ctrl_push() that
causes a NULL pointer dereference.

When a malformed GSP firmware message is received:

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 to r535_gsp_rpc_rm_ctrl_push():

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c:r535_gsp_rpc_rm_ctrl_push() {
    ...
	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);
	}
    ...
}

Does PTR_ERR(rpc) evaluate to 0 here when rpc is NULL? If so, this erroneously
signals success to callers like nvkm_gsp_rm_ctrl_rd(), which will then
blindly dereference the NULL pointer. Could this be handled differently so
an error code is returned instead?

[Severity: High]
This isn't a bug introduced by this patch, but looking at r535_gsp_rpc_push(),
could there be a memory leak of the RPC message buffer when an error occurs
while sending large payloads?

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c:r535_gsp_rpc_push() {
    ...
	if (payload_size > max_payload_size) {
    ...
		while (remain_payload_size) {
			u32 size = min(remain_payload_size,
				       max_payload_size);

			next = r535_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD, size);
			if (IS_ERR(next)) {
				repv = next;
				goto done;
			}
    ...
		if (!IS_ERR(repv))
			kvfree(msg);
	} else {
    ...
done:
	mutex_unlock(&gsp->cmdq.mutex);
	return repv;
}

When IS_ERR(next) is true during chunking, the execution jumps to the done
label, bypassing the kvfree(msg) cleanup logic. The caller function then sets
its pointer to NULL on error, losing the reference to the original msg
buffer. Does this result in a resource leak on this error path?

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