All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/nouveau: guard two render-node NULL derefs of absent NVKM subdevices
@ 2026-08-12 15:00 Zhenhao Wan
  2026-08-12 15:00 ` [PATCH 1/2] drm/nouveau/sw: prevent NULL deref of disp in vblank methods Zhenhao Wan
  2026-08-12 15:00 ` [PATCH 2/2] drm/nouveau: prevent NULL deref of gr in GETPARAM_GRAPH_UNITS Zhenhao Wan
  0 siblings, 2 replies; 5+ messages in thread
From: Zhenhao Wan @ 2026-08-12 15:00 UTC (permalink / raw)
  To: Lyude Paul, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: dri-devel, nouveau, linux-kernel, Yuhao Jiang, stable,
	Zhenhao Wan

Two NULL-pointer dereferences are reachable by an unprivileged local user
holding a /dev/dri/renderD* fd when an optional NVKM component is legitimately
absent after probe.

When a subdevice/engine constructor returns -ENODEV, the NVKM device
constructor (NVKM_LAYOUT_ONCE) treats it as an optional absent component: it
deletes the subdevice, leaves the device-> pointer NULL, and continues probing.
disp can also be NULLed by nvkm_subdev_disable() (strap-driven) on a headless
card. nouveau still completes init and unconditionally registers a render node,
so two method sinks that assume the subdevice is present become reachable:

  - the NV50/GF100 sw-class vblank methods load device->disp->vblank.index_nr
    (disp == NULL on a headless card);
  - GETPARAM_GRAPH_UNITS calls nvkm_gr_units(gr), which reads gr->func
    (gr == NULL when GR construction returned -ENODEV).

Either yields a NULL read -> kernel oops -> panic-class local DoS.

Both fixes extend tolerance that neighbouring code already proves is expected:
the sw channel constructors already guard disp, and nvkm_gr_units() already
returns 0 when the engine exposes no units callback. No new control-flow paths
are added.

There is no Fixes: tag: these are long-standing bugs predating the nvkm
reorg, so they are sent to stable without one.

---
Zhenhao Wan (2):
      drm/nouveau/sw: prevent NULL deref of disp in vblank methods
      drm/nouveau: prevent NULL deref of gr in GETPARAM_GRAPH_UNITS

 drivers/gpu/drm/nouveau/nouveau_abi16.c        | 2 +-
 drivers/gpu/drm/nouveau/nvkm/engine/sw/gf100.c | 2 +-
 drivers/gpu/drm/nouveau/nvkm/engine/sw/nv50.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
---
base-commit: db2ddb87143519e20a95aa36c60b36107b736a58
change-id: 20260812-nouveau-nvkm-absent-subdev-null-deref-f52db5c24408

Best regards,
--  
Zhenhao Wan <whi4ed0g@gmail.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] drm/nouveau/sw: prevent NULL deref of disp in vblank methods
  2026-08-12 15:00 [PATCH 0/2] drm/nouveau: guard two render-node NULL derefs of absent NVKM subdevices Zhenhao Wan
@ 2026-08-12 15:00 ` Zhenhao Wan
  2026-08-12 15:27   ` sashiko-bot
  2026-08-12 15:00 ` [PATCH 2/2] drm/nouveau: prevent NULL deref of gr in GETPARAM_GRAPH_UNITS Zhenhao Wan
  1 sibling, 1 reply; 5+ messages in thread
From: Zhenhao Wan @ 2026-08-12 15:00 UTC (permalink / raw)
  To: Lyude Paul, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: dri-devel, nouveau, linux-kernel, Yuhao Jiang, stable,
	Zhenhao Wan

The NV50 and GF100 software-class vblank methods nv50_sw_chan_mthd()
and gf100_sw_chan_mthd() evaluate "data < device->disp->vblank.index_nr"
to validate the requested vblank head before allowing its notifier.

On a headless card the display subdevice is absent: nvkm_subdev_disable()
(strap-driven from devinit) and the -ENODEV path of the NVKM_LAYOUT_ONCE
device constructor both leave device->disp == NULL while the device
continues to probe and still registers a render node (DRIVER_RENDER). An
unprivileged client holding a /dev/dri/renderD* fd can then create a
software channel and push method 0x0408 (NV50) / 0x040c (GF100), which
dereferences the NULL device->disp and oopses the kernel.

The channel constructors nv50_sw_chan_new() and gf100_sw_chan_new()
already tolerate an absent display ("for (i = 0; disp && ...)"), so no
vblank notifier is ever registered on such a card and the method handlers
are the only path that assumes disp is present. Guard the dereference
with the same disp NULL test the constructors already use.

Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Cc: stable@vger.kernel.org
Signed-off-by: Zhenhao Wan <whi4ed0g@gmail.com>
---
 drivers/gpu/drm/nouveau/nvkm/engine/sw/gf100.c | 2 +-
 drivers/gpu/drm/nouveau/nvkm/engine/sw/nv50.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/sw/gf100.c b/drivers/gpu/drm/nouveau/nvkm/engine/sw/gf100.c
index 0171cdf6f639..4cf8cd120c76 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) {
 			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 0cfb1eaae6de..b202e11238a5 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) {
 			nvkm_event_ntfy_allow(&chan->vblank.notify[data]);
 			return true;
 		}

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] drm/nouveau: prevent NULL deref of gr in GETPARAM_GRAPH_UNITS
  2026-08-12 15:00 [PATCH 0/2] drm/nouveau: guard two render-node NULL derefs of absent NVKM subdevices Zhenhao Wan
  2026-08-12 15:00 ` [PATCH 1/2] drm/nouveau/sw: prevent NULL deref of disp in vblank methods Zhenhao Wan
@ 2026-08-12 15:00 ` Zhenhao Wan
  2026-08-12 15:20   ` sashiko-bot
  1 sibling, 1 reply; 5+ messages in thread
From: Zhenhao Wan @ 2026-08-12 15:00 UTC (permalink / raw)
  To: Lyude Paul, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: dri-devel, nouveau, linux-kernel, Yuhao Jiang, stable,
	Zhenhao Wan

nouveau_abi16_ioctl_getparam() fetches the graphics engine with
gr = nvxx_gr(drm) (i.e. drm->nvkm->gr) and, for the
NOUVEAU_GETPARAM_GRAPH_UNITS parameter, passes it straight to
nvkm_gr_units(), which dereferences gr->func with no NULL check.

When graphics-engine construction returns -ENODEV, NVKM treats it as an
optional absent engine: the NVKM_LAYOUT_ONCE device constructor deletes
the subdevice, leaves device->gr == NULL and continues probing, and
Nouveau still registers a render node. An unprivileged client holding a
/dev/dri/renderD* fd can then issue DRM_IOCTL_NOUVEAU_GETPARAM with
NOUVEAU_GETPARAM_GRAPH_UNITS and oops the kernel on the NULL gr.

Report 0 units when gr is absent. This matches nvkm_gr_units() itself,
which already returns 0 when the engine exposes no units callback.

Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Cc: stable@vger.kernel.org
Signed-off-by: Zhenhao Wan <whi4ed0g@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_abi16.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index 291203121f0c..dd3f8d386377 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -306,7 +306,7 @@ nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
 		getparam->value = 1;
 		break;
 	case NOUVEAU_GETPARAM_GRAPH_UNITS:
-		getparam->value = nvkm_gr_units(gr);
+		getparam->value = gr ? nvkm_gr_units(gr) : 0;
 		break;
 	case NOUVEAU_GETPARAM_EXEC_PUSH_MAX: {
 		int ib_max = getparam_dma_ib_max(device);

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] drm/nouveau: prevent NULL deref of gr in GETPARAM_GRAPH_UNITS
  2026-08-12 15:00 ` [PATCH 2/2] drm/nouveau: prevent NULL deref of gr in GETPARAM_GRAPH_UNITS Zhenhao Wan
@ 2026-08-12 15:20   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-08-12 15:20 UTC (permalink / raw)
  To: Zhenhao Wan; +Cc: dri-devel

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

Pre-existing issues:
- [High] The patch fixes a NULL pointer dereference for `gr` in `nouveau_abi16_ioctl_getparam()`, but misses an identical vulnerability in `nouveau_abi16_ioctl_get_zcull_info()` within the same file.
--

--- Patch [2]: [PATCH 2/2] drm/nouveau: prevent NULL deref of gr in GETPARAM_GRAPH_UNITS ---
commit 368f46fbd7701fc98a09aa941af876abf6e518b8
Author: Zhenhao Wan <whi4ed0g@gmail.com>

drm/nouveau: prevent NULL deref of gr in GETPARAM_GRAPH_UNITS

This patch prevents a NULL pointer dereference in nouveau_abi16_ioctl_getparam()
when an unprivileged client requests NOUVEAU_GETPARAM_GRAPH_UNITS. If the
graphics engine is absent, gr is NULL, which previously oopsed the kernel.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> index 291203121f0c2..dd3f8d386377f 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> @@ -306,7 +306,7 @@ nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
>  		getparam->value = 1;
>  		break;
>  	case NOUVEAU_GETPARAM_GRAPH_UNITS:
> -		getparam->value = nvkm_gr_units(gr);
> +		getparam->value = gr ? nvkm_gr_units(gr) : 0;
>  		break;
>  	case NOUVEAU_GETPARAM_EXEC_PUSH_MAX: {
>  		int ib_max = getparam_dma_ib_max(device);

[Severity: High]
This is a pre-existing issue, but does a similar NULL pointer dereference
vulnerability exist slightly further down in the same file?

If the graphics engine is absent (making gr == NULL), could an unprivileged
client calling DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO trigger a kernel oops here?

drivers/gpu/drm/nouveau/nouveau_abi16.c:nouveau_abi16_ioctl_get_zcull_info() {
	...
	struct nvkm_gr *gr = nvxx_gr(drm);
	struct drm_nouveau_get_zcull_info *out = data;

	if (gr->has_zcull_info) {
	...
}

Should nouveau_abi16_ioctl_get_zcull_info() also check if gr is valid before
dereferencing it?

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] drm/nouveau/sw: prevent NULL deref of disp in vblank methods
  2026-08-12 15:00 ` [PATCH 1/2] drm/nouveau/sw: prevent NULL deref of disp in vblank methods Zhenhao Wan
@ 2026-08-12 15:27   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-08-12 15:27 UTC (permalink / raw)
  To: Zhenhao Wan; +Cc: dri-devel

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 <whi4ed0g@gmail.com>

    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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-12 15:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12 15:00 [PATCH 0/2] drm/nouveau: guard two render-node NULL derefs of absent NVKM subdevices Zhenhao Wan
2026-08-12 15:00 ` [PATCH 1/2] drm/nouveau/sw: prevent NULL deref of disp in vblank methods Zhenhao Wan
2026-08-12 15:27   ` sashiko-bot
2026-08-12 15:00 ` [PATCH 2/2] drm/nouveau: prevent NULL deref of gr in GETPARAM_GRAPH_UNITS Zhenhao Wan
2026-08-12 15:20   ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.