Re: [PATCH v4 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:
- [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