Dan Carpenter
2025-May-23 16:03 UTC
[bug report] drm/nouveau/gsp: add hal for gsp.get_static_info()
[ Did these files get renamed or something? No idea why the warnings are showing up as new now. ] Hello Ben Skeggs, Commit 7bb77eacdb85 ("drm/nouveau/gsp: add hal for gsp.get_static_info()") from Nov 14, 2024 (linux-next), leads to the following Smatch static checker warning: drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c:47 r535_gsp_rpc_rm_ctrl_push() warn: passing zero to 'PTR_ERR' drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c:223 r535_gsp_get_static_info() warn: 'rpc' can also be NULL drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c:90 r570_gsp_get_static_info() warn: 'rpc' can also be NULL drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c 212 static int 213 r535_gsp_get_static_info(struct nvkm_gsp *gsp) 214 { 215 GspStaticConfigInfo *rpc; 216 217 rpc = nvkm_gsp_rpc_rd(gsp, NV_VGPU_MSG_FUNCTION_GET_GSP_STATIC_INFO, sizeof(*rpc)); 218 if (IS_ERR(rpc)) 219 return PTR_ERR(rpc); 220 221 gsp->internal.client.object.client = &gsp->internal.client; 222 gsp->internal.client.object.parent = NULL; --> 223 gsp->internal.client.object.handle = rpc->hInternalClient; ^^^^^ The nvkm_gsp_rpc_rd() function returns a mix of error pointers and NULL but if it returns NULL then obviously this dereference will crash. https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/ regards, dan carpenter
Timur Tabi
2025-May-23 20:53 UTC
[bug report] drm/nouveau/gsp: add hal for gsp.get_static_info()
On Fri, 2025-05-23 at 19:03 +0300, Dan Carpenter wrote:> [ Did these files get renamed or something?? No idea why the warnings > ? are showing up as new now. ]Ben posted a large refactor of the code last week.> Hello Ben Skeggs, > > Commit 7bb77eacdb85 ("drm/nouveau/gsp: add hal for > gsp.get_static_info()") from Nov 14, 2024 (linux-next), leads to the > following Smatch static checker warning: > > drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c:47 r535_gsp_rpc_rm_ctrl_push() warn: > passing zero to 'PTR_ERR' > drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c:223 r535_gsp_get_static_info() warn: 'rpc' > can also be NULL > drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c:90 r570_gsp_get_static_info() warn: 'rpc' > can also be NULL > > drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c > ??? 212 static int > ??? 213 r535_gsp_get_static_info(struct nvkm_gsp *gsp) > ??? 214 { > ??? 215???????? GspStaticConfigInfo *rpc; > ??? 216 > ??? 217???????? rpc = nvkm_gsp_rpc_rd(gsp, NV_VGPU_MSG_FUNCTION_GET_GSP_STATIC_INFO, > sizeof(*rpc)); > ??? 218???????? if (IS_ERR(rpc)) > ??? 219???????????????? return PTR_ERR(rpc); > ??? 220 > ??? 221???????? gsp->internal.client.object.client = &gsp->internal.client; > ??? 222???????? gsp->internal.client.object.parent = NULL; > --> 223???????? gsp->internal.client.object.handle = rpc->hInternalClient; > ???????????????????????????????????????????????????? ^^^^^ > > The nvkm_gsp_rpc_rd() function returns a mix of error pointers and NULL but > if it returns NULL then obviously this dereference will crash. > > https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/Well, there's definitely something weird going on with this code. It appears that nvkm_gsp_rpc_rd() actually returns NULL on success. ? nvkm_gsp_rpc_rd -> r535_gsp_rpc_push -> r535_gsp_rpc_handle_reply() So either I'm confused, or this will need further debugging.