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

[lyude]

On Fri, 2026-10-02 at 12:09 -0600, Jim Cromie via B4 Relay wrote:
> From: Jim Cromie <jim.cromie@xxxxxxxxx>
> 
> When hardware or firmware initialization fails, the graphics engine
> (gr) or device functions may remain NULL. Attempting to access these
> during the GETPARAM ioctl (e.g., NOUVEAU_GETPARAM_GRAPH_UNITS)
> results
> in a kernel NULL pointer dereference, causing a crash when userspace
> (GNOME/Mesa) attempts to probe the device.
> 
> Add safety checks for 'gr', 'gr->func', and 'nvkm_device->func' in
> the
> ioctl handler. Return -ENODEV to signal the missing hardware state to
> userspace, and use NV_ERROR_ONCE to provide diagnostic proof in the
> kernel log without risking a console flood.
> 
> NOTE:
> 
> I encountered these crashes while booting unrelated code (in
> dynamic-debug) on real HW.  There was also some fwupd updates around
> this time, so it could all be transient/unreproducible.  But it did
> happen, and could happen again in unforseen circumstances.
> 
> Signed-off-by: Jim Cromie <jim.cromie@xxxxxxxxx>
> ---
>  drivers/gpu/drm/nouveau/nouveau_abi16.c | 25 +++++++++++++++++++++--
> --
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c
> b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> index 4542d5f4ded8..cb1810dee8e5 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;
>  	case NOUVEAU_GETPARAM_EXEC_PUSH_MAX: {
>  		int ib_max = getparam_dma_ib_max(device);
> @@ -315,11 +320,23 @@ nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
>  		break;
>  	}
>  	case NOUVEAU_GETPARAM_VRAM_BAR_SIZE:
> -		getparam->value = nvkm_device->func-
> >resource_size(nvkm_device, NVKM_BAR1_FB);
> +		if (nvkm_device && nvkm_device->func && nvkm_device-
> >func->resource_size) {
> +			getparam->value =
> +				nvkm_device->func-
> >resource_size(nvkm_device, NVKM_BAR1_FB);
> +		} else {
> +			NV_ERROR_ONCE(drm, "GETPARAM_VRAM_BAR_SIZE:
> no device func\n");
> +			return -ENODEV;
> +		}

For both of these I would just invert the conditionals like this:

if (!nvkm_device ||
    !nvkm_device->func ||
    !nvkm_device->func->resource_size) {
	NV_ERROR_ONCE(drm, "blah blah");
	return -ENODEV;
}


>  		break;
>  	case NOUVEAU_GETPARAM_VRAM_USED: {
> -		struct ttm_resource_manager *vram_mgr =
> ttm_manager_type(&drm->ttm.bdev, TTM_PL_VRAM);
> -		getparam->value =
> (u64)ttm_resource_manager_usage(vram_mgr);
> +		struct ttm_resource_manager *vram_mgr =
> +			ttm_manager_type(&drm->ttm.bdev,
> TTM_PL_VRAM);
> +		if (vram_mgr) {
> +			getparam->value =
> (u64)ttm_resource_manager_usage(vram_mgr);
> +		} else {
> +			NV_ERROR_ONCE(drm, "GETPARAM_VRAM_USED: no
> vram mgr\n");
> +			return -ENODEV;
> +		}
>  		break;
>  	}
>  	case NOUVEAU_GETPARAM_HAS_VMA_TILEMODE: