Re: [PATCH 1/2] drm/nouveau/sw: prevent NULL deref of disp in vblank methods

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

Pre-existing issues:
- [Critical] Potential heap buffer overflow and out-of-bounds array access in SW channel initialization and method handlers if the display hardware supports more than 4 heads.
- [High] Truncation of 64-bit virtual addresses on hardware with >40-bit address spaces (e.g., GV100) when updating vblank offset.
--

--- Patch [1]: [PATCH 1/2] drm/nouveau/sw: prevent NULL deref of disp in vblank methods ---
commit 8a4bbb200908564b29f376aa02dccafae5c8262f
Author: Zhenhao Wan <[email protected]>

    drm/nouveau/sw: prevent NULL deref of disp in vblank methods

This commit prevents a NULL pointer dereference in the software-class vblank
methods nv50_sw_chan_mthd() and gf100_sw_chan_mthd(). It adds a check to verify
device->disp is not NULL before accessing device->disp->vblank.index_nr,
protecting against kernel oopses on headless cards where the display subdevice
is absent.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/sw/gf100.c b/drivers/gpu/drm/nouveau/nvkm/engine/sw/gf100.c
> index 0171cdf6f6397..4cf8cd120c765 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/sw/gf100.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/sw/gf100.c
> @@ -72,7 +72,7 @@ gf100_sw_chan_mthd(struct nvkm_sw_chan *base, int subc, u32 mthd, u32 data)
>  		chan->vblank.value = data;
>  		return true;
>  	case 0x040c:
> -		if (data < device->disp->vblank.index_nr) {
> +		if (device->disp && data < device->disp->vblank.index_nr) {

[Severity: Critical]
This is a pre-existing issue, but can this out-of-bounds array access cause
heap corruption?

The notify array in struct nv50_sw_chan is fixed to 4 elements:

	struct nvkm_event_ntfy notify[4];

If the hardware supports more than 4 display heads (e.g. GV100) and
device->disp->vblank.index_nr is greater than 4, a user-supplied data value
up to index_nr - 1 can bypass the check and overflow the notify array:

	nvkm_event_ntfy_allow(&chan->vblank.notify[data]);

Could we bound-check data against the array capacity instead of index_nr?


[Severity: High]
This isn't a bug introduced by this patch, but does the offset handling
earlier in gf100_sw_chan_mthd() truncate 64-bit virtual addresses?

When userspace attempts to set a virtual address greater than 40 bits and
writes method 0x0400 followed by 0x0404, it looks like bits 40-63 of the
offset are erroneously cleared by the mask:

	case 0x0404:
		chan->vblank.offset &= 0xff00000000ULL;
		chan->vblank.offset |= data;

This clears bits above 39, which could cause the vblank semaphore to be
written to an incorrect GPU virtual address.

>  			nvkm_event_ntfy_allow(&chan->vblank.notify[data]);
>  			return true;
>  		}
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/sw/nv50.c b/drivers/gpu/drm/nouveau/nvkm/engine/sw/nv50.c
> index 0cfb1eaae6ded..b202e11238a53 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/sw/nv50.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/sw/nv50.c
> @@ -69,7 +69,7 @@ nv50_sw_chan_mthd(struct nvkm_sw_chan *base, int subc, u32 mthd, u32 data)
>  	case 0x0400: chan->vblank.offset = data; return true;
>  	case 0x0404: chan->vblank.value  = data; return true;
>  	case 0x0408:
> -		if (data < device->disp->vblank.index_nr) {
> +		if (device->disp && data < device->disp->vblank.index_nr) {

[Severity: Critical]
This is a pre-existing issue, but this location suffers from the same
out-of-bounds array access as gf100_sw_chan_mthd().

Since chan->vblank.notify is a 4-element array, if
device->disp->vblank.index_nr exceeds 4, could an unprivileged client
provide a data value that causes an overflow when accessing
&chan->vblank.notify[data]?

>  			nvkm_event_ntfy_allow(&chan->vblank.notify[data]);
>  			return true;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812-nouveau-nvkm-absent-subdev-null-deref-v1-0-7e057f6aeba0@gmail.com?part=1