[PATCH] drm/nouveau: disable VM_BIND once a legacy BO VMA is created

[PATCH] drm/nouveau: disable VM_BIND once a legacy BO VMA is created

[Zhenhao Wan]

nouveau_gem_object_open() creates a legacy per-client VMA when the client
has not initialised VM_BIND (!nouveau_cli_uvmm()), but
nouveau_gem_object_close() only tears it down while the client is still not
a VM_BIND client ("if (nouveau_cli_uvmm(cli)) return;").

nouveau_gem_ioctl_new() calls nouveau_cli_disable_uvmm_noinit() to stop a
client mixing the legacy and VM_BIND uAPIs, but the prime-import and flink
GEM_OPEN paths that also run nouveau_gem_object_open() do not. A client can
thus open an object (creating a legacy VMA), call NOUVEAU_VM_INIT, then
close the handle: close() now returns early and leaks the VMA. If the BO is
flink-shared it outlives the client's freed vmm, and the next migration
walks nvbo->vma_list in nouveau_bo_move_ntfy() and dereferences the stale
vma->vmm -- a use-after-free.

Disable VM_BIND in nouveau_gem_object_open() as nouveau_gem_ioctl_new()
does; the helper is a no-op once VM_BIND is initialised, so VM_BIND-first
clients are unaffected.

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_gem.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 20dba02d6175..e33f76b5a239 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -108,6 +108,14 @@ nouveau_gem_object_open(struct drm_gem_object *gem, struct drm_file *file_priv)
 	if (vmm->vmm.object.oclass < NVIF_CLASS_VMM_NV50)
 		return 0;
 
+	/*
+	 * Opening the object binds it through the legacy per-VMA path below.
+	 * Disable VM_BIND for this client, as nouveau_gem_ioctl_new() does, so
+	 * a later NOUVEAU_VM_INIT cannot switch the client to uvmm and leave
+	 * nouveau_gem_object_close() unable to tear this VMA down.
+	 */
+	nouveau_cli_disable_uvmm_noinit(cli);
+
 	if (nvbo->no_share && uvmm &&
 	    drm_gpuvm_resv(&uvmm->base) != nvbo->bo.base.resv)
 		return -EPERM;

---
base-commit: db2ddb87143519e20a95aa36c60b36107b736a58
change-id: 20260813-nouveau-gem-uvmm-vma-fix-8d0fbac454b9

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

Re: [PATCH] drm/nouveau: disable VM_BIND once a legacy BO VMA is created

[sashiko-bot]

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

Pre-existing issues:
- [High] A time-of-check to time-of-use (TOCTOU) race condition in `nouveau_gem_object_open()` allows a VM_BIND client to bypass the `NOUVEAU_GEM_DOMAIN_NO_SHARE` restriction and open exclusive buffers.
- [High] Unprivileged userspace can trigger a massive memory and GPU resource leak by repeatedly calling the `NOUVEAU_VM_INIT` ioctl.
--

--- Patch [1]: [PATCH] drm/nouveau: disable VM_BIND once a legacy BO VMA is created ---
commit f241cc2db54abcee53ca3ff7b4e2aadd6db117dd
Author: Zhenhao Wan <whi4ed0g@gmail.com>

drm/nouveau: disable VM_BIND once a legacy BO VMA is created

This commit fixes a use-after-free issue where a legacy per-client VMA could
be leaked if a client initializes VM_BIND after opening an object. It calls
nouveau_cli_disable_uvmm_noinit() in nouveau_gem_object_open() to disable
VM_BIND for clients creating legacy VMAs.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
> index 20dba02d6175b..e33f76b5a2398 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_gem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
> @@ -108,6 +108,14 @@ nouveau_gem_object_open(struct drm_gem_object *gem, struct drm_file *file_priv)
>  	if (vmm->vmm.object.oclass < NVIF_CLASS_VMM_NV50)
>  		return 0;
>  
> +	/*
> +	 * Opening the object binds it through the legacy per-VMA path below.
> +	 * Disable VM_BIND for this client, as nouveau_gem_ioctl_new() does, so
> +	 * a later NOUVEAU_VM_INIT cannot switch the client to uvmm and leave
> +	 * nouveau_gem_object_close() unable to tear this VMA down.
> +	 */
> +	nouveau_cli_disable_uvmm_noinit(cli);
> +
>  	if (nvbo->no_share && uvmm &&
>  	    drm_gpuvm_resv(&uvmm->base) != nvbo->bo.base.resv)
>  		return -EPERM;

[Severity: High]
This is a pre-existing issue, but does this function have a time-of-check to
time-of-use race condition involving the cached uvmm pointer?

The uvmm pointer is cached without locks at the start of
nouveau_gem_object_open():

	struct nouveau_uvmm *uvmm = nouveau_cli_uvmm(cli);

If a client guesses the flink name of a no_share buffer, calls GEM_OPEN, and
concurrently calls NOUVEAU_VM_INIT:

CPU1 calls NOUVEAU_VM_INIT and sets cli->uvmm.ptr

CPU2 in nouveau_gem_object_open():
	struct nouveau_uvmm *uvmm = nouveau_cli_uvmm(cli); /* caches NULL */
	...
	nouveau_cli_disable_uvmm_noinit(cli); /* locks and disables */

	if (nvbo->no_share && uvmm &&
	    drm_gpuvm_resv(&uvmm->base) != nvbo->bo.base.resv)
		return -EPERM;

Can NOUVEAU_VM_INIT complete after uvmm is cached as NULL but before
nouveau_cli_disable_uvmm_noinit() acquires the lock?

If so, the no_share condition would be evaluated using the stale NULL pointer,
bypassing the restriction and allowing a client to map an exclusive buffer
belonging to another VM into its own GPU VM. Should uvmm be reloaded after
taking the lock or before the check?


[Severity: High]
This isn't a bug introduced by this patch, but can unprivileged userspace
trigger a massive memory and GPU resource leak through the VM_BIND ioctl?

Looking at nouveau_uvmm_ioctl_vm_init() in
drivers/gpu/drm/nouveau/nouveau_uvmm.c:

	uvmm = kzalloc_obj(*uvmm);
	if (!uvmm) {
		ret = -ENOMEM;
		goto out_unlock;
	}
...
	cli->uvmm.ptr = uvmm;

If an attacker repeatedly calls DRM_IOCTL_NOUVEAU_VM_INIT on a single DRM file
descriptor, does this ioctl handler lack a check to see if cli->uvmm.ptr is
already initialized?

It appears to unconditionally allocate a new struct nouveau_uvmm and overwrite
cli->uvmm.ptr. Does this permanently leak the previously allocated structures
and GPU page tables, potentially leading to a local denial of service?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813-nouveau-gem-uvmm-vma-fix-v1-1-4f14d427f2c9@gmail.com?part=1