mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/3] drm/nouveau/uvmm: fix VM_BIND submit/unwind error-path bugs
@ 2026-08-11  8:46 Zhenhao Wan
  2026-08-11  8:46 ` [PATCH v2 1/3] drm/nouveau/uvmm: fix NULL deref unwinding an OP_MAP_SPARSE op Zhenhao Wan
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Zhenhao Wan @ 2026-08-11  8:46 UTC (permalink / raw)
  To: Lyude Paul, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Dave Airlie
  Cc: dri-devel, nouveau, linux-kernel, Zhenhao Wan, Yuhao Jiang, stable

Three bugs on the nouveau VM_BIND submit/unwind/cleanup error path, all
introduced by the original VM_BIND uAPI (Fixes: b88baab82871) and all
reachable by an unprivileged render-node client:

  1/3 NULL deref when the unwind loop frees a never-populated
      OP_MAP_SPARSE op's ops list.
  2/3 premature free of a region on a failed OP_UNMAP_SPARSE, because
      op->reg is left set on the failure paths.
  3/3 a sparse region left permanently marked dirty after unwind,
      wedging later binds over that range.

Each fix is minimal and self-contained; all are Cc: stable.

Signed-off-by: Zhenhao Wan <whi4ed0g@gmail.com>
---
Changes in v2:
- Add an Assisted-by: trailer to each patch; no code changes.
- Link to v1: https://patch.msgid.link/20260811-nouveau-uvmm-vmbind-fixes-v1-0-40df1c046b49@gmail.com

---
Zhenhao Wan (3):
      drm/nouveau/uvmm: fix NULL deref unwinding an OP_MAP_SPARSE op
      drm/nouveau/uvmm: fix premature region free on failed OP_UNMAP_SPARSE
      drm/nouveau/uvmm: clear the dirty flag when unwinding an OP_UNMAP_SPARSE

 drivers/gpu/drm/nouveau/nouveau_uvmm.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
---
base-commit: ba3e43a9e601636f5edb54e259a74f96ca3b8fd8
change-id: 20260811-nouveau-uvmm-vmbind-fixes-c2dfab359df2

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


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

* [PATCH v2 1/3] drm/nouveau/uvmm: fix NULL deref unwinding an OP_MAP_SPARSE op
  2026-08-11  8:46 [PATCH v2 0/3] drm/nouveau/uvmm: fix VM_BIND submit/unwind error-path bugs Zhenhao Wan
@ 2026-08-11  8:46 ` Zhenhao Wan
  2026-08-11 21:26   ` lyude
  2026-08-11  8:46 ` [PATCH v2 2/3] drm/nouveau/uvmm: fix premature region free on failed OP_UNMAP_SPARSE Zhenhao Wan
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Zhenhao Wan @ 2026-08-11  8:46 UTC (permalink / raw)
  To: Lyude Paul, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Dave Airlie
  Cc: dri-devel, nouveau, linux-kernel, Zhenhao Wan, Yuhao Jiang, stable

Each bind_job_op is zeroed by kzalloc_obj() in bind_job_op_from_uop(),
and the OP_MAP_SPARSE case in nouveau_uvmm_bind_job_submit() only creates
a region, so op->ops stays NULL for a successfully processed sparse map.

If a later op in the same job fails, the reverse unwind loop revisits that
op and calls drm_gpuva_ops_free(&uvmm->base, op->ops) unconditionally.
drm_gpuva_ops_free() dereferences its argument right away
(list_for_each_entry_safe on &ops->list), so a NULL op->ops oopses. The
path is reachable by any render-node fd holder, since NOUVEAU_VM_BIND is
DRM_RENDER_ALLOW.

Guard the free with IS_ERR_OR_NULL(), as nouveau_uvmm_bind_job_cleanup()
already does for the identical free.

Fixes: b88baab82871 ("drm/nouveau: implement new VM_BIND uAPI")
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_uvmm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index 36445915aa58..849bf42c124e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -1489,7 +1489,8 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job,
 			break;
 		}
 
-		drm_gpuva_ops_free(&uvmm->base, op->ops);
+		if (!IS_ERR_OR_NULL(op->ops))
+			drm_gpuva_ops_free(&uvmm->base, op->ops);
 		op->ops = NULL;
 		op->reg = NULL;
 	}

-- 
2.34.1


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

* [PATCH v2 2/3] drm/nouveau/uvmm: fix premature region free on failed OP_UNMAP_SPARSE
  2026-08-11  8:46 [PATCH v2 0/3] drm/nouveau/uvmm: fix VM_BIND submit/unwind error-path bugs Zhenhao Wan
  2026-08-11  8:46 ` [PATCH v2 1/3] drm/nouveau/uvmm: fix NULL deref unwinding an OP_MAP_SPARSE op Zhenhao Wan
@ 2026-08-11  8:46 ` Zhenhao Wan
  2026-08-11  8:46 ` [PATCH v2 3/3] drm/nouveau/uvmm: clear the dirty flag when unwinding an OP_UNMAP_SPARSE Zhenhao Wan
  2026-08-11 21:56 ` [PATCH v2 0/3] drm/nouveau/uvmm: fix VM_BIND submit/unwind error-path bugs lyude
  3 siblings, 0 replies; 6+ messages in thread
From: Zhenhao Wan @ 2026-08-11  8:46 UTC (permalink / raw)
  To: Lyude Paul, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Dave Airlie
  Cc: dri-devel, nouveau, linux-kernel, Zhenhao Wan, Yuhao Jiang, stable

In nouveau_uvmm_bind_job_submit()'s OP_UNMAP_SPARSE arm, op->reg is set
from nouveau_uvma_region_find(), which only looks the region up and takes
no reference; a region's sole reference is its membership in
uvmm->region_mt. Two failure paths leave op->reg set: the -ENOENT check
when the region is busy, and the drm_gpuvm_sm_unmap_ops_create() failure.
The sibling nouveau_uvmm_sm_unmap_prepare() failure just below clears
op->reg; these two do not.

unwind_continue steps back one op, so the failing op is skipped by the
unwind loop and its op->reg stays set. nouveau_uvmm_bind_job_cleanup()
then enters its if (op->reg) branch and calls nouveau_uvma_region_remove()
and nouveau_uvma_region_put() on it, dropping the tree's sole reference
and freeing a region this job never created. The comment above the
cleanup loop documents the broken invariant: op->reg must be NULL on
submit failure.

This frees a live region on an unrelated failure, reachable single-job
when drm_gpuvm_sm_unmap_ops_create() returns -ENOMEM; if another job owns
the same region, its cleanup then removes and puts the freed region, a
use-after-free. Clear op->reg on both failure paths.

Fixes: b88baab82871 ("drm/nouveau: implement new VM_BIND uAPI")
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_uvmm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index 849bf42c124e..3412200b21fd 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -1319,6 +1319,7 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job,
 							   op->va.range);
 			if (!op->reg || op->reg->dirty) {
 				ret = -ENOENT;
+				op->reg = NULL;
 				goto unwind_continue;
 			}
 
@@ -1327,6 +1328,7 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job,
 								op->va.range);
 			if (IS_ERR(op->ops)) {
 				ret = PTR_ERR(op->ops);
+				op->reg = NULL;
 				goto unwind_continue;
 			}
 

-- 
2.34.1


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

* [PATCH v2 3/3] drm/nouveau/uvmm: clear the dirty flag when unwinding an OP_UNMAP_SPARSE
  2026-08-11  8:46 [PATCH v2 0/3] drm/nouveau/uvmm: fix VM_BIND submit/unwind error-path bugs Zhenhao Wan
  2026-08-11  8:46 ` [PATCH v2 1/3] drm/nouveau/uvmm: fix NULL deref unwinding an OP_MAP_SPARSE op Zhenhao Wan
  2026-08-11  8:46 ` [PATCH v2 2/3] drm/nouveau/uvmm: fix premature region free on failed OP_UNMAP_SPARSE Zhenhao Wan
@ 2026-08-11  8:46 ` Zhenhao Wan
  2026-08-11 21:56 ` [PATCH v2 0/3] drm/nouveau/uvmm: fix VM_BIND submit/unwind error-path bugs lyude
  3 siblings, 0 replies; 6+ messages in thread
From: Zhenhao Wan @ 2026-08-11  8:46 UTC (permalink / raw)
  To: Lyude Paul, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Dave Airlie
  Cc: dri-devel, nouveau, linux-kernel, Zhenhao Wan, Yuhao Jiang, stable

A successful OP_UNMAP_SPARSE marks its region dirty with
nouveau_uvma_region_dirty() and defers the teardown to
nouveau_uvmm_bind_job_cleanup(); it does not remove the region from
uvmm->region_mt.

If a later op in the job fails, the unwind path never clears reg->dirty
(set in one place, cleared nowhere) and sets op->reg = NULL, so cleanup
skips the teardown. The region is left in the tree with dirty set and its
completion never signalled. Later binds over that range then fail
permanently -- -ENOENT or -EINVAL from the dirty checks, or an unkillable
wait_for_completion() in bind_validate_region() -- for the lifetime of
the uvmm.

Clear reg->dirty when the unwind reverts the sparse unmap, restoring the
region to the state it was found in.

Fixes: b88baab82871 ("drm/nouveau: implement new VM_BIND uAPI")
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_uvmm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index 3412200b21fd..76ab377f0e2c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -1475,6 +1475,7 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job,
 						    op->va.range);
 			break;
 		case OP_UNMAP_SPARSE:
+			op->reg->dirty = false;
 			__nouveau_uvma_region_insert(uvmm, op->reg);
 			nouveau_uvmm_sm_unmap_prepare_unwind(uvmm, &op->new,
 							     op->ops);

-- 
2.34.1


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

* Re: [PATCH v2 1/3] drm/nouveau/uvmm: fix NULL deref unwinding an OP_MAP_SPARSE op
  2026-08-11  8:46 ` [PATCH v2 1/3] drm/nouveau/uvmm: fix NULL deref unwinding an OP_MAP_SPARSE op Zhenhao Wan
@ 2026-08-11 21:26   ` lyude
  0 siblings, 0 replies; 6+ messages in thread
From: lyude @ 2026-08-11 21:26 UTC (permalink / raw)
  To: Zhenhao Wan, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Dave Airlie
  Cc: dri-devel, nouveau, linux-kernel, Yuhao Jiang, stable

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Tue, 2026-08-11 at 16:46 +0800, Zhenhao Wan wrote:
> Each bind_job_op is zeroed by kzalloc_obj() in
> bind_job_op_from_uop(),
> and the OP_MAP_SPARSE case in nouveau_uvmm_bind_job_submit() only
> creates
> a region, so op->ops stays NULL for a successfully processed sparse
> map.
> 
> If a later op in the same job fails, the reverse unwind loop revisits
> that
> op and calls drm_gpuva_ops_free(&uvmm->base, op->ops)
> unconditionally.
> drm_gpuva_ops_free() dereferences its argument right away
> (list_for_each_entry_safe on &ops->list), so a NULL op->ops oopses.
> The
> path is reachable by any render-node fd holder, since NOUVEAU_VM_BIND
> is
> DRM_RENDER_ALLOW.
> 
> Guard the free with IS_ERR_OR_NULL(), as
> nouveau_uvmm_bind_job_cleanup()
> already does for the identical free.
> 
> Fixes: b88baab82871 ("drm/nouveau: implement new VM_BIND uAPI")
> 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_uvmm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> index 36445915aa58..849bf42c124e 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> @@ -1489,7 +1489,8 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job
> *job,
>  			break;
>  		}
>  
> -		drm_gpuva_ops_free(&uvmm->base, op->ops);
> +		if (!IS_ERR_OR_NULL(op->ops))
> +			drm_gpuva_ops_free(&uvmm->base, op->ops);
>  		op->ops = NULL;
>  		op->reg = NULL;
>  	}


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

* Re: [PATCH v2 0/3] drm/nouveau/uvmm: fix VM_BIND submit/unwind error-path bugs
  2026-08-11  8:46 [PATCH v2 0/3] drm/nouveau/uvmm: fix VM_BIND submit/unwind error-path bugs Zhenhao Wan
                   ` (2 preceding siblings ...)
  2026-08-11  8:46 ` [PATCH v2 3/3] drm/nouveau/uvmm: clear the dirty flag when unwinding an OP_UNMAP_SPARSE Zhenhao Wan
@ 2026-08-11 21:56 ` lyude
  3 siblings, 0 replies; 6+ messages in thread
From: lyude @ 2026-08-11 21:56 UTC (permalink / raw)
  To: Zhenhao Wan, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Dave Airlie
  Cc: dri-devel, nouveau, linux-kernel, Yuhao Jiang, stable

This patch series is:

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Tue, 2026-08-11 at 16:46 +0800, Zhenhao Wan wrote:
> Three bugs on the nouveau VM_BIND submit/unwind/cleanup error path,
> all
> introduced by the original VM_BIND uAPI (Fixes: b88baab82871) and all
> reachable by an unprivileged render-node client:
> 
>   1/3 NULL deref when the unwind loop frees a never-populated
>       OP_MAP_SPARSE op's ops list.
>   2/3 premature free of a region on a failed OP_UNMAP_SPARSE, because
>       op->reg is left set on the failure paths.
>   3/3 a sparse region left permanently marked dirty after unwind,
>       wedging later binds over that range.
> 
> Each fix is minimal and self-contained; all are Cc: stable.
> 
> Signed-off-by: Zhenhao Wan <whi4ed0g@gmail.com>
> ---
> Changes in v2:
> - Add an Assisted-by: trailer to each patch; no code changes.
> - Link to v1:
> https://patch.msgid.link/20260811-nouveau-uvmm-vmbind-fixes-v1-0-40df1c046b49@gmail.com
> 
> ---
> Zhenhao Wan (3):
>       drm/nouveau/uvmm: fix NULL deref unwinding an OP_MAP_SPARSE op
>       drm/nouveau/uvmm: fix premature region free on failed
> OP_UNMAP_SPARSE
>       drm/nouveau/uvmm: clear the dirty flag when unwinding an
> OP_UNMAP_SPARSE
> 
>  drivers/gpu/drm/nouveau/nouveau_uvmm.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> ---
> base-commit: ba3e43a9e601636f5edb54e259a74f96ca3b8fd8
> change-id: 20260811-nouveau-uvmm-vmbind-fixes-c2dfab359df2
> 
> Best regards,
> --  
> Zhenhao Wan <whi4ed0g@gmail.com>


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

end of thread, other threads:[~2026-08-11 21:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-11  8:46 [PATCH v2 0/3] drm/nouveau/uvmm: fix VM_BIND submit/unwind error-path bugs Zhenhao Wan
2026-08-11  8:46 ` [PATCH v2 1/3] drm/nouveau/uvmm: fix NULL deref unwinding an OP_MAP_SPARSE op Zhenhao Wan
2026-08-11 21:26   ` lyude
2026-08-11  8:46 ` [PATCH v2 2/3] drm/nouveau/uvmm: fix premature region free on failed OP_UNMAP_SPARSE Zhenhao Wan
2026-08-11  8:46 ` [PATCH v2 3/3] drm/nouveau/uvmm: clear the dirty flag when unwinding an OP_UNMAP_SPARSE Zhenhao Wan
2026-08-11 21:56 ` [PATCH v2 0/3] drm/nouveau/uvmm: fix VM_BIND submit/unwind error-path bugs lyude

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®