Re: [PATCH 1/2] drm/nouveau: Fix NULL pointer dereferences in GETPARAM ioctl

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] The patch fixes a NULL pointer dereference in the `GETPARAM` ioctl when the graphics engine (`gr`) fails to initialize but misses an identical issue in `nouveau_abi16_ioctl_get_zcull_info`.
--

--- Patch [1]: [PATCH 1/2] drm/nouveau: Fix NULL pointer dereferences in GETPARAM ioctl ---
commit 6569813c05cdc1cf39cfd469b6745e81e7cad86d
Author: Jim Cromie <jim.cromie@gmail.com>

drm/nouveau: Fix NULL pointer dereferences in GETPARAM ioctl

This patch adds safety checks for the graphics engine and device functions
in the GETPARAM ioctl handler. It returns -ENODEV to signal missing
hardware state to userspace, preventing a kernel NULL pointer dereference
when initialization fails.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> index 291203121f0c2..c9270c5b0fac8 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> @@ -306,7 +306,12 @@ nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
>  		getparam->value = 1;
>  		break;
>  	case NOUVEAU_GETPARAM_GRAPH_UNITS:
> -		getparam->value = nvkm_gr_units(gr);
> +		if (gr && gr->func) {
> +			getparam->value = nvkm_gr_units(gr);
> +		} else {
> +			NV_ERROR_ONCE(drm, "GETPARAM_GRAPH_UNITS: no gr engine or func\n");
> +			return -ENODEV;
> +		}
>  		break;

[Severity: Critical]
This is a pre-existing issue, but while you are fixing the NULL pointer
dereference of the gr pointer here, is there a similar vulnerability
in the adjacent nouveau_abi16_ioctl_get_zcull_info() function?

If the graphics engine fails to initialize and gr is NULL, as addressed by
this patch, it looks like an unprivileged userspace process could still
trigger an Oops by calling the DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO ioctl,
which only requires DRM_RENDER_ALLOW.

Looking at the nouveau_abi16_ioctl_get_zcull_info() function in
drivers/gpu/drm/nouveau/nouveau_abi16.c, the gr pointer is unconditionally
dereferenced:

int
nouveau_abi16_ioctl_get_zcull_info(ABI16_IOCTL_ARGS)
{
	struct nouveau_drm *drm = nouveau_drm(dev);
	struct nvkm_gr *gr = nvxx_gr(drm);
	struct drm_nouveau_get_zcull_info *out = data;

	if (gr->has_zcull_info) {
		const struct nvkm_gr_zcull_info *i = &gr->zcull_info;
		...

Would it make sense to add a similar check for gr being NULL before
dereferencing gr->has_zcull_info?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815-null-fixes-v1-0-0df9dda951fb@gmail.com?part=1