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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Medium] The original GSP message wrapper (`msg`) is leaked on error paths when pushing large RPC payloads.
--

commit 983901af13a13a0d204f00f3cb9a175b176bf22f
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: Medium]
This isn't a bug introduced by this patch, but does the GSP message wrapper
leak on error paths when pushing large RPC payloads?

In 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;
		}
		...
		repv = r535_gsp_rpc_send(gsp, next, NVKM_GSP_RPC_REPLY_NOWAIT, 0);
		if (IS_ERR(repv))
			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 {

If a payload exceeds max_payload_size, the original msg wrapper is retained
while the payload is sent in chunks using new messages. The chunks are
properly freed by the inner r535_gsp_cmdq_push(), but the original msg is
only freed at the end if no error occurred.

If an error occurs at any point (e.g., r535_gsp_rpc_get() fails,
r535_gsp_rpc_send() fails, or the final reply is an error pointer), the
function exits without freeing msg.

Callers such as r535_gsp_rpc_rm_ctrl_push() clear their pointers on error:

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

so they cannot free the message either, which appears to result in a memory
leak on these error paths.

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