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 <[email protected]> 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/[email protected]?part=4
