[PATCH 0/2] drm/nouveau: fix unbounded kernel allocations in the VM_BIND uAPI

[PATCH 0/2] drm/nouveau: fix unbounded kernel allocations in the VM_BIND uAPI

[Junrui Luo via B4 Relay]

Two independent fixes to the uAPI added by b88baab82871 ("drm/nouveau:
implement new VM_BIND uAPI").

Patch 1 bounds the sync and operation counts in EXEC and VM_BIND.
nouveau_exec_ucopy() and nouveau_uvmm_vm_bind_ucopy() hand user-supplied
u32 counts straight to u_memcpya(), which only rejects multiplication
overflow -- something a u32 count times a small element size never
reaches on 64-bit.  A wait_count of 0xffffffff thus becomes a 64 GB
vmemdup_user() request: above INT_MAX that trips the WARN_ON_ONCE() in
__kvmalloc_node_noprof(), and below it the kernel attempts a vmalloc of
up to 2 GB that GFP_USER leaves uncharged to the caller's memcg.

Patch 2 rejects a second VM_INIT.  nouveau_uvmm_ioctl_vm_init() never
checks whether the client already has a GPU VA space before overwriting
cli->uvmm.ptr, so a second call orphans the first nouveau_uvmm with no
remaining reference to it.  The orphan, its drm_gpuvm, that gpuvm's
reservation object and its region maple tree are never freed, the buffer
objects mapped in it stay pinned, and its nvif vmm keeps the GPU page
directories allocated until the file is closed.

Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
Junrui Luo (2):
      drm/nouveau: bound sync and op counts in EXEC and VM_BIND
      drm/nouveau/uvmm: reject a second VM_INIT

 drivers/gpu/drm/nouveau/nouveau_exec.c | 12 ++++++++++++
 drivers/gpu/drm/nouveau/nouveau_uvmm.c | 24 ++++++++++++++++++++++++
 include/uapi/drm/nouveau_drm.h         | 18 ++++++++++++++++++
 3 files changed, 54 insertions(+)
---
base-commit: 075b74841bd0065a3bda3440873c747938e69b68
change-id: 20260808-nouveau-fixes-70a70f3f5385

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>

[PATCH 1/2] drm/nouveau: bound sync and op counts in EXEC and VM_BIND

[Junrui Luo via B4 Relay]

From: Junrui Luo <moonafterrain@outlook.com>

nouveau_exec_ucopy() and nouveau_uvmm_vm_bind_ucopy() pass user-supplied
u32 counts to u_memcpya(). DRM_IOCTL_NOUVEAU_EXEC bounds only
req->push_count against push_max, leaving req->wait_count and
req->sig_count unchecked; DRM_IOCTL_NOUVEAU_VM_BIND bounds none of
op_count, wait_count or sig_count. u_memcpya() itself only rejects
multiplication overflow, which on 64-bit never triggers for a u32 count
times a small element size.

A wait_count of 0xffffffff therefore becomes a 64 GB vmemdup_user()
request. Since vmemdup_user() allocates with GFP_USER and hence without
__GFP_NOWARN, a size above INT_MAX trips the WARN_ON_ONCE() in
__kvmalloc_node_noprof(); below that the kernel attempts an up to 2 GB
vmalloc that GFP_USER also leaves uncharged to the caller's memcg. Both
ioctls are DRM_RENDER_ALLOW, so any client holding a render node can
issue this.

Reject the oversized counts at the ioctl entry points, the way
nouveau_gem_ioctl_pushbuf() and the existing push_count check already
do, so that the client is told which limit it exceeded. Sync objects get
NOUVEAU_MAX_SYNCS, matching both NOUVEAU_GEM_MAX_BUFFERS and the value
xe settled on for the same field in DRM_XE_MAX_SYNCS. VM_BIND operations
have no comparable semantic limit, so NOUVEAU_VM_BIND_MAX_OPS is set
well above any batch size a client is expected to submit; it exists only
to keep the copy-in allocation finite.

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: Junrui Luo <moonafterrain@outlook.com>
---
 drivers/gpu/drm/nouveau/nouveau_exec.c | 12 ++++++++++++
 drivers/gpu/drm/nouveau/nouveau_uvmm.c | 18 ++++++++++++++++++
 include/uapi/drm/nouveau_drm.h         | 18 ++++++++++++++++++
 3 files changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.c b/drivers/gpu/drm/nouveau/nouveau_exec.c
index a08ab1cfea9b..7bdccbae53b1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_exec.c
+++ b/drivers/gpu/drm/nouveau/nouveau_exec.c
@@ -389,6 +389,18 @@ nouveau_exec_ioctl_exec(struct drm_device *dev,
 		return nouveau_abi16_put(abi16, -EINVAL);
 	}
 
+	if (unlikely(req->wait_count > NOUVEAU_MAX_SYNCS)) {
+		NV_PRINTK(err, cli, "exec wait count exceeds limit: %d max %d\n",
+			  req->wait_count, NOUVEAU_MAX_SYNCS);
+		return nouveau_abi16_put(abi16, -EINVAL);
+	}
+
+	if (unlikely(req->sig_count > NOUVEAU_MAX_SYNCS)) {
+		NV_PRINTK(err, cli, "exec sig count exceeds limit: %d max %d\n",
+			  req->sig_count, NOUVEAU_MAX_SYNCS);
+		return nouveau_abi16_put(abi16, -EINVAL);
+	}
+
 	ret = nouveau_exec_ucopy(&args, req);
 	if (ret)
 		goto out;
diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index f5e4756b4de4..bced1481674e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -1807,6 +1807,24 @@ nouveau_uvmm_ioctl_vm_bind(struct drm_device *dev,
 	if (unlikely(!nouveau_cli_uvmm_locked(cli)))
 		return -ENOSYS;
 
+	if (unlikely(req->op_count > NOUVEAU_VM_BIND_MAX_OPS)) {
+		NV_PRINTK(err, cli, "vm_bind op count exceeds limit: %d max %d\n",
+			  req->op_count, NOUVEAU_VM_BIND_MAX_OPS);
+		return -EINVAL;
+	}
+
+	if (unlikely(req->wait_count > NOUVEAU_MAX_SYNCS)) {
+		NV_PRINTK(err, cli, "vm_bind wait count exceeds limit: %d max %d\n",
+			  req->wait_count, NOUVEAU_MAX_SYNCS);
+		return -EINVAL;
+	}
+
+	if (unlikely(req->sig_count > NOUVEAU_MAX_SYNCS)) {
+		NV_PRINTK(err, cli, "vm_bind sig count exceeds limit: %d max %d\n",
+			  req->sig_count, NOUVEAU_MAX_SYNCS);
+		return -EINVAL;
+	}
+
 	ret = nouveau_uvmm_vm_bind_ucopy(&args, req);
 	if (ret)
 		return ret;
diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h
index 1fa82fa6af38..35ddf97ca873 100644
--- a/include/uapi/drm/nouveau_drm.h
+++ b/include/uapi/drm/nouveau_drm.h
@@ -220,6 +220,14 @@ struct drm_nouveau_gem_cpu_fini {
 	__u32 handle;
 };
 
+/*
+ * NOUVEAU_MAX_SYNCS - maximum number of sync objects per ioctl
+ *
+ * The maximum value EXEC and VM_BIND accept in their wait_count and
+ * sig_count fields.
+ */
+#define NOUVEAU_MAX_SYNCS 1024
+
 /**
  * struct drm_nouveau_sync - sync object
  *
@@ -332,6 +340,16 @@ struct drm_nouveau_vm_bind_op {
 	__u64 range;
 };
 
+/*
+ * NOUVEAU_VM_BIND_MAX_OPS - maximum number of &drm_nouveau_vm_bind_ops
+ *
+ * The maximum value VM_BIND accepts in its op_count field. There is no
+ * semantic limit on the number of operations a bind may carry; this bound
+ * exists only to keep the copy-in allocation finite and is far above any
+ * batch size a client is expected to submit.
+ */
+#define NOUVEAU_VM_BIND_MAX_OPS 65536
+
 /**
  * struct drm_nouveau_vm_bind - structure for DRM_IOCTL_NOUVEAU_VM_BIND
  */

-- 
2.51.2

Re: [PATCH 1/2] drm/nouveau: bound sync and op counts in EXEC and VM_BIND

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Un-rate-limited error logging creates a potential log-spam DoS vector.
- [Medium] Validation of flag-gated syscall parameters outside their flag scope.
- [Low] Format string specifier mismatch in error logging for __u32 counts.
--

--- Patch [1]: [PATCH 1/2] drm/nouveau: bound sync and op counts in EXEC and VM_BIND ---
commit 6d0adda3ff0dd4a0e4440dc92362fcb0ad993d9a
Author: Junrui Luo <moonafterrain@outlook.com>
Subject: drm/nouveau: bound sync and op counts in EXEC and VM_BIND

This commit introduces upper bounds for sync and op counts in the EXEC and
VM_BIND ioctls. By restricting the size of these counts, it prevents potential
excessive memory allocations that could trigger warnings or exhaust memory.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.c b/drivers/gpu/drm/nouveau/nouveau_exec.c
> index a08ab1cfea9be..7bdccbae53b1d 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_exec.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_exec.c
> @@ -389,6 +389,18 @@ nouveau_exec_ioctl_exec(struct drm_device *dev,
>  		return nouveau_abi16_put(abi16, -EINVAL);
>  	}
>  
> +	if (unlikely(req->wait_count > NOUVEAU_MAX_SYNCS)) {
> +		NV_PRINTK(err, cli, "exec wait count exceeds limit: %d max %d\n",
> +			  req->wait_count, NOUVEAU_MAX_SYNCS);

[Severity: Low]
Are the format string specifiers correct here? The req->wait_count field is
a __u32 type, so using %d will print large values as negative numbers.

[Severity: Medium]
Could this un-rate-limited NV_PRINTK log be intentionally triggered by
userspace to spam the kernel log buffer and hide other messages?

> +		return nouveau_abi16_put(abi16, -EINVAL);
> +	}
> +
> +	if (unlikely(req->sig_count > NOUVEAU_MAX_SYNCS)) {
> +		NV_PRINTK(err, cli, "exec sig count exceeds limit: %d max %d\n",
> +			  req->sig_count, NOUVEAU_MAX_SYNCS);
> +		return nouveau_abi16_put(abi16, -EINVAL);
> +	}
> +
>  	ret = nouveau_exec_ucopy(&args, req);

[ ... ]

> diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> index f5e4756b4de4a..bced1481674e7 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> @@ -1807,6 +1807,24 @@ nouveau_uvmm_ioctl_vm_bind(struct drm_device *dev,
>  	if (unlikely(!nouveau_cli_uvmm_locked(cli)))
>  		return -ENOSYS;
>  
> +	if (unlikely(req->op_count > NOUVEAU_VM_BIND_MAX_OPS)) {
> +		NV_PRINTK(err, cli, "vm_bind op count exceeds limit: %d max %d\n",
> +			  req->op_count, NOUVEAU_VM_BIND_MAX_OPS);
> +		return -EINVAL;
> +	}
> +
> +	if (unlikely(req->wait_count > NOUVEAU_MAX_SYNCS)) {

[Severity: Medium]
According to the userspace API documentation, sync objects are only accepted
when DRM_NOUVEAU_VM_BIND_RUN_ASYNC is set in req->flags. If a user submits a
VM_BIND ioctl without this flag, might wait_count and sig_count contain
uninitialized garbage from userspace stack slots?

If so, is it safe to validate these flag-gated parameters outside the scope
of their flag check, rather than explicitly ignoring or rejecting them when
the flag is absent?

> +		NV_PRINTK(err, cli, "vm_bind wait count exceeds limit: %d max %d\n",
> +			  req->wait_count, NOUVEAU_MAX_SYNCS);
> +		return -EINVAL;
> +	}
> +
> +	if (unlikely(req->sig_count > NOUVEAU_MAX_SYNCS)) {
> +		NV_PRINTK(err, cli, "vm_bind sig count exceeds limit: %d max %d\n",
> +			  req->sig_count, NOUVEAU_MAX_SYNCS);
> +		return -EINVAL;
> +	}
> +
>  	ret = nouveau_uvmm_vm_bind_ucopy(&args, req);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260808-nouveau-fixes-v1-0-c3ebdc17a89c@outlook.com?part=1

Re: [PATCH 1/2] drm/nouveau: bound sync and op counts in EXEC and VM_BIND

[David Airlie]

> From: Junrui Luo <moonafterrain@outlook.com>
>
> nouveau_exec_ucopy() and nouveau_uvmm_vm_bind_ucopy() pass user-supplied
> u32 counts to u_memcpya(). DRM_IOCTL_NOUVEAU_EXEC bounds only
> req->push_count against push_max, leaving req->wait_count and
> req->sig_count unchecked; DRM_IOCTL_NOUVEAU_VM_BIND bounds none of
> op_count, wait_count or sig_count. u_memcpya() itself only rejects
> multiplication overflow, which on 64-bit never triggers for a u32 count
> times a small element size.
>
> A wait_count of 0xffffffff therefore becomes a 64 GB vmemdup_user()
> request. Since vmemdup_user() allocates with GFP_USER and hence without
> __GFP_NOWARN, a size above INT_MAX trips the WARN_ON_ONCE() in
> __kvmalloc_node_noprof(); below that the kernel attempts an up to 2 GB
> vmalloc that GFP_USER also leaves uncharged to the caller's memcg. Both
> ioctls are DRM_RENDER_ALLOW, so any client holding a render node can
> issue this.
>
> Reject the oversized counts at the ioctl entry points, the way
> nouveau_gem_ioctl_pushbuf() and the existing push_count check already
> do, so that the client is told which limit it exceeded. Sync objects get
> NOUVEAU_MAX_SYNCS, matching both NOUVEAU_GEM_MAX_BUFFERS and the value
> xe settled on for the same field in DRM_XE_MAX_SYNCS. VM_BIND operations
> have no comparable semantic limit, so NOUVEAU_VM_BIND_MAX_OPS is set
> well above any batch size a client is expected to submit; it exists only
> to keep the copy-in allocation finite.
>
> 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: Junrui Luo <moonafterrain@outlook.com>

Have you run a complete Vulkan CTS with NVK across this with no regressions?

I'm weary of those limits being too low, but I think the only app that
might push them beyond CTS would be Forza Horizon 5 or 6.

Dave.


> ---
>  drivers/gpu/drm/nouveau/nouveau_exec.c | 12 ++++++++++++
>  drivers/gpu/drm/nouveau/nouveau_uvmm.c | 18 ++++++++++++++++++
>  include/uapi/drm/nouveau_drm.h         | 18 ++++++++++++++++++
>  3 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.c b/drivers/gpu/drm/nouveau/nouveau_exec.c
> index a08ab1cfea9b..7bdccbae53b1 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_exec.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_exec.c
> @@ -389,6 +389,18 @@ nouveau_exec_ioctl_exec(struct drm_device *dev,
>                 return nouveau_abi16_put(abi16, -EINVAL);
>         }
>
> +       if (unlikely(req->wait_count > NOUVEAU_MAX_SYNCS)) {
> +               NV_PRINTK(err, cli, "exec wait count exceeds limit: %d max %d\n",
> +                         req->wait_count, NOUVEAU_MAX_SYNCS);
> +               return nouveau_abi16_put(abi16, -EINVAL);
> +       }
> +
> +       if (unlikely(req->sig_count > NOUVEAU_MAX_SYNCS)) {
> +               NV_PRINTK(err, cli, "exec sig count exceeds limit: %d max %d\n",
> +                         req->sig_count, NOUVEAU_MAX_SYNCS);
> +               return nouveau_abi16_put(abi16, -EINVAL);
> +       }
> +
>         ret = nouveau_exec_ucopy(&args, req);
>         if (ret)
>                 goto out;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> index f5e4756b4de4..bced1481674e 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> @@ -1807,6 +1807,24 @@ nouveau_uvmm_ioctl_vm_bind(struct drm_device *dev,
>         if (unlikely(!nouveau_cli_uvmm_locked(cli)))
>                 return -ENOSYS;
>
> +       if (unlikely(req->op_count > NOUVEAU_VM_BIND_MAX_OPS)) {
> +               NV_PRINTK(err, cli, "vm_bind op count exceeds limit: %d max %d\n",
> +                         req->op_count, NOUVEAU_VM_BIND_MAX_OPS);
> +               return -EINVAL;
> +       }
> +
> +       if (unlikely(req->wait_count > NOUVEAU_MAX_SYNCS)) {
> +               NV_PRINTK(err, cli, "vm_bind wait count exceeds limit: %d max %d\n",
> +                         req->wait_count, NOUVEAU_MAX_SYNCS);
> +               return -EINVAL;
> +       }
> +
> +       if (unlikely(req->sig_count > NOUVEAU_MAX_SYNCS)) {
> +               NV_PRINTK(err, cli, "vm_bind sig count exceeds limit: %d max %d\n",
> +                         req->sig_count, NOUVEAU_MAX_SYNCS);
> +               return -EINVAL;
> +       }
> +
>         ret = nouveau_uvmm_vm_bind_ucopy(&args, req);
>         if (ret)
>                 return ret;
> diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h
> index 1fa82fa6af38..35ddf97ca873 100644
> --- a/include/uapi/drm/nouveau_drm.h
> +++ b/include/uapi/drm/nouveau_drm.h
> @@ -220,6 +220,14 @@ struct drm_nouveau_gem_cpu_fini {
>         __u32 handle;
>  };
>
> +/*
> + * NOUVEAU_MAX_SYNCS - maximum number of sync objects per ioctl
> + *
> + * The maximum value EXEC and VM_BIND accept in their wait_count and
> + * sig_count fields.
> + */
> +#define NOUVEAU_MAX_SYNCS 1024
> +
>  /**
>   * struct drm_nouveau_sync - sync object
>   *
> @@ -332,6 +340,16 @@ struct drm_nouveau_vm_bind_op {
>         __u64 range;
>  };
>
> +/*
> + * NOUVEAU_VM_BIND_MAX_OPS - maximum number of &drm_nouveau_vm_bind_ops
> + *
> + * The maximum value VM_BIND accepts in its op_count field. There is no
> + * semantic limit on the number of operations a bind may carry; this bound
> + * exists only to keep the copy-in allocation finite and is far above any
> + * batch size a client is expected to submit.
> + */
> +#define NOUVEAU_VM_BIND_MAX_OPS 65536
> +
>  /**
>   * struct drm_nouveau_vm_bind - structure for DRM_IOCTL_NOUVEAU_VM_BIND
>   */
>
> --
> 2.51.2
>
>

Re: [PATCH 1/2] drm/nouveau: bound sync and op counts in EXEC and VM_BIND

[Junrui Luo]

On Mon, Aug 10, 2026 at 08:43:50AM +1000, David Airlie wrote:
> Have you run a complete Vulkan CTS with NVK across this with no regressions?
> 
> I'm weary of those limits being too low, but I think the only app that
> might push them beyond CTS would be Forza Horizon 5 or 6.

No, I have not. I don't currently have the hardware to run NVK, so I can't
offer a CTS run or any other regression test for this.

Is there a better guard that avoids a hard limit here, or does this need
a test before it can go in?

Thanks,
Junrui Luo

Re: [PATCH 1/2] drm/nouveau: bound sync and op counts in EXEC and VM_BIND

[Dave Airlie]

On Tue, 11 Aug 2026 at 14:00, Junrui Luo <moonafterrain@outlook.com> wrote:
>
> On Mon, Aug 10, 2026 at 08:43:50AM +1000, David Airlie wrote:
> > Have you run a complete Vulkan CTS with NVK across this with no regressions?
> >
> > I'm weary of those limits being too low, but I think the only app that
> > might push them beyond CTS would be Forza Horizon 5 or 6.
>
> No, I have not. I don't currently have the hardware to run NVK, so I can't
> offer a CTS run or any other regression test for this.
>
> Is there a better guard that avoids a hard limit here, or does this need
> a test before it can go in?

Looking at xe it also has 1024 limit on number of syncs, but it
doesn't have any limit on number of binds,

                *bind_ops = kvmalloc_objs(struct drm_xe_vm_bind_op,
                                          args->num_binds,
                                          GFP_KERNEL | __GFP_ACCOUNT |
__GFP_RETRY_MAYFAIL | __GFP_NOWARN);

Maybe nouveau could do the same.

Dave.
>
> Thanks,
> Junrui Luo

Re: [PATCH 1/2] drm/nouveau: bound sync and op counts in EXEC and VM_BIND

[Danilo Krummrich]

On 8/11/26 6:05 AM, Dave Airlie wrote:
> On Tue, 11 Aug 2026 at 14:00, Junrui Luo <moonafterrain@outlook.com> wrote:
>>
>> On Mon, Aug 10, 2026 at 08:43:50AM +1000, David Airlie wrote:
>>> Have you run a complete Vulkan CTS with NVK across this with no regressions?
>>>
>>> I'm weary of those limits being too low, but I think the only app that
>>> might push them beyond CTS would be Forza Horizon 5 or 6.
>>
>> No, I have not. I don't currently have the hardware to run NVK, so I can't
>> offer a CTS run or any other regression test for this.
>>
>> Is there a better guard that avoids a hard limit here, or does this need
>> a test before it can go in?
> 
> Looking at xe it also has 1024 limit on number of syncs, but it
> doesn't have any limit on number of binds,
> 
>                 *bind_ops = kvmalloc_objs(struct drm_xe_vm_bind_op,
>                                           args->num_binds,
>                                           GFP_KERNEL | __GFP_ACCOUNT |
> __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> 
> Maybe nouveau could do the same.
SGTM, I'd also be hesitant to put a hard limit on VM_BIND.

@Junrui: Can you please resend?

[PATCH 2/2] drm/nouveau/uvmm: reject a second VM_INIT

[Junrui Luo via B4 Relay]

From: Junrui Luo <moonafterrain@outlook.com>

nouveau_uvmm_ioctl_vm_init() sets up the GPU VA space for a drm_file and
is reachable from an unprivileged render node client (DRM_RENDER_ALLOW).
After the cli->uvmm.disabled check it unconditionally allocates a
nouveau_uvmm, initialises its drm_gpuvm and region maple tree, creates
the backing nvif vmm and overwrites cli->uvmm.ptr, without testing
whether one already exists.

Calling DRM_IOCTL_NOUVEAU_VM_INIT twice therefore drops the previous
nouveau_uvmm with no remaining reference to it: drm_gpuvm_put() is
reached only from nouveau_uvmm_fini(), which nouveau_cli_fini() calls
once on whatever cli->uvmm.ptr holds at close time. The orphaned
nouveau_uvmm, its drm_gpuvm, that gpuvm's reservation GEM object and its
region maple tree are never freed, buffer objects mapped in it stay
pinned by the orphaned uvmas, and its nvif vmm keeps the GPU page
directories allocated until the file is closed, so repeating the ioctl
leaks kernel memory without bound.

Test cli->uvmm.ptr under cli->mutex before anything is allocated and
return -EBUSY, mirroring nouveau_svmm_init().

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: Junrui Luo <moonafterrain@outlook.com>
---
 drivers/gpu/drm/nouveau/nouveau_uvmm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index bced1481674e..26d2a57b5aac 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -1929,6 +1929,12 @@ nouveau_uvmm_ioctl_vm_init(struct drm_device *dev,
 		goto out_unlock;
 	}
 
+	/* Check that a GPU VA space isn't already set up for the client. */
+	if (cli->uvmm.ptr) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
 	uvmm = kzalloc_obj(*uvmm);
 	if (!uvmm) {
 		ret = -ENOMEM;

-- 
2.51.2

Re: [PATCH 2/2] drm/nouveau/uvmm: reject a second VM_INIT

[sashiko-bot]

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

Pre-existing issues:
- [High] Lockless access to `cli->uvmm.ptr` lacks memory barriers, potentially allowing readers to observe a partially initialized `uvmm` struct.
--

--- Patch [2]: [PATCH 2/2] drm/nouveau/uvmm: reject a second VM_INIT ---
commit 8aef81c8581a1b964e410ee072207a76a4098339
Author: Junrui Luo <moonafterrain@outlook.com>

drm/nouveau/uvmm: reject a second VM_INIT

nouveau_uvmm_ioctl_vm_init() sets up the GPU VA space for a drm_file and
is reachable from an unprivileged render node client. The patch prevents
DRM_IOCTL_NOUVEAU_VM_INIT from dropping the previous nouveau_uvmm and
leaking kernel memory by checking if cli->uvmm.ptr is already set.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> index bced1481674e7..26d2a57b5aacc 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> @@ -1929,6 +1929,12 @@ nouveau_uvmm_ioctl_vm_init(struct drm_device *dev,
>  		goto out_unlock;
>  	}
>  
> +	/* Check that a GPU VA space isn't already set up for the client. */
> +	if (cli->uvmm.ptr) {
> +		ret = -EBUSY;
> +		goto out_unlock;
> +	}
> +

[Severity: High]
This is a pre-existing issue, but since we are checking cli->uvmm.ptr here,
is it safe that the writer publishes this pointer locklessly later in this
same function?

Further down in nouveau_uvmm_ioctl_vm_init(), the pointer is assigned without
memory barriers:

	uvmm->vmm.cli = cli;
	cli->uvmm.ptr = uvmm;
	mutex_unlock(&cli->mutex);

Could the compiler or CPU reorder this store before the uvmm structure is
fully initialized, causing readers to see a partially initialized struct?

For example, concurrent calls to nouveau_cli_uvmm() load cli->uvmm.ptr without
READ_ONCE() or smp_load_acquire():

drivers/gpu/drm/nouveau/nouveau_drv.h:nouveau_cli_uvmm() {
	return cli ? cli->uvmm.ptr : NULL;
}

If another thread executes DRM_IOCTL_MODE_CREATE_DUMB, it can follow a lockless
path through nouveau_gem_new() into nouveau_bo_alloc() which dereferences the
vmm structure:

drivers/gpu/drm/nouveau/nouveau_bo.c:nouveau_bo_alloc() {
	...
	struct nvif_vmm *vmm = &nouveau_cli_vmm(cli)->vmm;
	...
		for (i = 0; i < vmm->page_nr; i++) {
	...
}

Alternatively, nouveau_debugfs_gpuva() iterates clients and locklessly reads
cli->uvmm.ptr, then immediately calls nouveau_uvmm_lock() on it. Could this
trigger a crash if the mutex appears uninitialized?

Would using smp_store_release() for the assignment and smp_load_acquire()
for the readers ensure proper visibility of the initialized structure?

>  	uvmm = kzalloc_obj(*uvmm);
>  	if (!uvmm) {
>  		ret = -ENOMEM;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260808-nouveau-fixes-v1-0-c3ebdc17a89c@outlook.com?part=2