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-7e057f6ae...@gmail.com?part=1
