Re: [PATCH] GPU: drm: nouveau: Prevent kernel oops on missing firmware binary

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/nouveau: redundant gr NULL check in nvkm_gr_units() after caller fix
--

commit f535382c50460cf93f6d91dd265e009e3ef99a0a
Author: Jonathan Kuehn <jonathan.kuehn@aol.com>

GPU: drm: nouveau: Prevent kernel oops on missing firmware binary

This commit adds a NULL check for the gr pointer in nvkm_gr_units() to avoid
a NULL pointer dereference when querying NOUVEAU_GETPARAM_GRAPH_UNITS with
missing firmware binaries.

>     GPU: drm: nouveau: Prevent kernel oops on missing firmware binary
>     
>     Prevent null pointer dereference at the call of
>     nouveau_abi16_ioctl_getparams with parameter
>     NOUVEAU_GETPARAM_GRAPH_UNITS when firmware binaries are missing.
>     Fixes crash for certain user-space applications (steamwebhelper,
>     signal-desktop).
>     
>     Signed-off-by: Jonathan Kuehn <jonathan.kuehn@aol.com>
> 
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/gr/base.c b/drivers/gpu/drm/nouveau/nvkm/engine/gr/base.c
> index cd4908b1b4dff..9abeb9732406f 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/gr/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/gr/base.c
> @@ -72,7 +72,7 @@ nvkm_gr_tile(struct nvkm_engine *engine, int region, struct nvkm_fb_tile *tile)
>  u64
>  nvkm_gr_units(struct nvkm_gr *gr)
>  {
> -	if (gr->func->units)
> +	if (gr && gr->func->units)

[Severity: Medium]
Can a NULL gr pointer still reach nvkm_gr_units() here?

Commit f569473c1393 ("drm/nouveau: prevent NULL deref of gr in
GETPARAM_GRAPH_UNITS") already added a NULL check in
nouveau_abi16_ioctl_getparam(), which is the sole caller of nvkm_gr_units()
in the tree:

drivers/gpu/drm/nouveau/nouveau_abi16.c:nouveau_abi16_ioctl_getparam() {
	...
	case NOUVEAU_GETPARAM_GRAPH_UNITS:
		getparam->value = gr ? nvkm_gr_units(gr) : 0;
		break;
	...
}

Since nouveau_abi16_ioctl_getparam() already guards against gr being NULL when
handling NOUVEAU_GETPARAM_GRAPH_UNITS, does adding a second check in
nvkm_gr_units() without updating the caller just duplicate the check with no
change in runtime behavior?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105202757.28703-1-jonathan.kuehn@aol.com?part=1