Re: [PATCH v3 03/10] drm/nouveau/disp: route GSP-RM display MMIO through nvkm_disp_func hooks

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

Pre-existing issues:
- [High] NULL pointer dereference and Use-After-Free on initialization failure in `r535_disp_new`.
--

--- Patch [3]: [PATCH v3 03/10] drm/nouveau/disp: route GSP-RM display MMIO through nvkm_disp_func hooks ---
commit b67572d820daaaf9db8443eb286700e054288fb6
Author: Mohamed Ahmed <[email protected]>

drm/nouveau/disp: route GSP-RM display MMIO through nvkm_disp_func hooks

This patch refactors the GSP-RM display MMIO routing by introducing a
gsp table in nvkm_disp_func. This allows r535_disp_new() to call hooks
specific to each chip generation rather than relying on hardcoded
functions. The changes remove chip-specific register code from
rm/r535/disp.c without altering functionality.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
> index cd4451e625122..bf97edcdfc959 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
[ ... ]
> @@ -1688,6 +1692,7 @@ r535_disp_new(const struct nvkm_disp_func *hw, struct nvkm_device *device,
>  	rm->uevent = hw->uevent;
>  	rm->sor.cnt = r535_sor_cnt;
>  	rm->sor.new = r535_sor_new;
> +	rm->gsp = hw->gsp;
>  	rm->ramht_size = hw->ramht_size;
>  
>  	rm->root.oclass = gpu->disp.class.root;
[ ... ]
>  	ret = nvkm_disp_new_(rm, device, type, inst, pdisp);
>  	if (ret)
>  		kfree(rm);
>  
>  	mutex_init(&(*pdisp)->super.mutex); //XXX

[Severity: High]
This is a pre-existing issue, but I noticed a potential NULL pointer
dereference and use-after-free here on initialization failure.

If nvkm_disp_new_() fails early due to an allocation failure, it can leave
*pdisp set to NULL. Does this cause mutex_init(&(*pdisp)->super.mutex) to
unconditionally dereference NULL?

Additionally, if nvkm_disp_new_() fails after linking the subdev to the
device, freeing rm with kfree(rm) could leave a dangling disp->func pointer.
During teardown, could the system invoke the destructor via this freed
pointer, resulting in a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3