Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Silently dropping oversized GSP RPC messages and reporting false success. -- commit c03df51ffa049e6fe56eec7d75137d404fe0855b Author: Hongling Zeng <[email protected]> nouveau/gsp/rm/alloc: Cleanup IS_ERR_OR_NULL usage This commit cleans up error checking by using IS_ERR() instead of WARN_ON(IS_ERR_OR_NULL()) after nvkm_gsp_rpc_get() calls, 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 > --- 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; > rpc->params.hObjectParent = 0; > rpc->params.hObjectOld = object->handle; > return nvkm_gsp_rpc_wr(gsp, rpc, NVKM_GSP_RPC_REPLY_RECV); [Severity: High] This isn't a bug introduced by this patch, but since this commit is cleaning up error handling around GSP RPC interactions, does nvkm_gsp_rpc_wr() also have an error handling issue that hides critical failures? Looking at drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c, if the GSP sends an RPC message where rpc->length > max_rpc_size, r535_gsp_msgq_recv() returns NULL without advancing the message queue read pointer: r535_gsp_msgq_recv() ... if (WARN_ON(rpc->length > max_rpc_size)) return NULL; This NULL propagates to nvkm_gsp_rpc_wr() in drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h, which translates it into a false success: nvkm_gsp_rpc_wr() ... if (IS_ERR(repv)) return PTR_ERR(repv); return 0; Since repv is NULL, IS_ERR(repv) evaluates to false, causing the function to silently return 0. Could this cause the caller to proceed assuming the RPC succeeded, potentially leading to an invalid state or use-after-free, while the unadvanced read pointer permanently wedges the message queue for all subsequent reads? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
