Re: [PATCH V1] accel/amdxdna: Drop dma-buf wrapping for ubuf

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/amdxdna: long-term mm_users reference leak in UBUF BO
--

commit 8ad12f14d7bf542056a227118a3d6363ee9a9c90
Author: Lizhi Hou <lizhi.hou@amd.com>

accel/amdxdna: Drop dma-buf wrapping for ubuf

This commit makes ubuf a DRM GEM private object and tracks the user VA with
HMM instead of a long-term pin. It creates the BO directly from the VA table
without going through dma-buf.

> diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.c b/drivers/accel/amdxdna/amdxdna_ubuf.c
> index 0e0cd69cd1fbf..cff5d319ad605 100644
> --- a/drivers/accel/amdxdna/amdxdna_ubuf.c
> +++ b/drivers/accel/amdxdna/amdxdna_ubuf.c
> @@ -35,16 +32,13 @@ static void amdxdna_gem_ubuf_obj_free(struct drm_gem_object *gobj)
>  {
> +	struct amdxdna_gem_obj *abo = to_xdna_obj(gobj);
> +
> +	amdxdna_hmm_unreg_umaps(abo, true);
> +	cancel_work_sync(&abo->hmm_unreg_work);
> +
> +	if (abo->mem.mm)
> +		mmput(abo->mem.mm);

[Severity: High]
Does this code leak the process's entire virtual address space if the BO
outlives the creating process?

A process calling the create BO ioctl to create a ubuf BO uses
mmget_not_zero() in amdxdna_alloc_ubuf_bo() to store an mm_users reference
on the creating process's mm. 

If the process then forks or passes the DRM file descriptor to another
process and exits, the GEM object outlives the original process. The mm_users
reference is held here until the BO is freed, preventing __mmput() from
executing and tearing down the original address space (VMAs, page tables,
anonymous memory). 

Could a malicious application repeatedly create processes, allocate ubuf BOs,
pass the DRM fd to a daemon, and exit, rapidly exhausting system memory?

> +	drm_gem_object_release(gobj);
> +	amdxdna_gem_destroy_obj(abo);
>  }

[ ... ]

> @@ -91,31 +95,28 @@ struct amdxdna_gem_obj *amdxdna_alloc_ubuf_bo(struct amdxdna_client *client,
> +	abo->type = AMDXDNA_BO_SHARE;
> +	abo->mem.uva = va_ent->vaddr;
> +	to_gobj(abo)->funcs = &amdxdna_gem_ubuf_obj_funcs;
> +	drm_gem_private_object_init(&xdna->ddev, to_gobj(abo), bufsize);
> +
> +	if (!mmget_not_zero(client->mm)) {
> +		ret = -EFAULT;
> +		goto put_obj;
> +	}
> +
> +	abo->mem.mm = client->mm;

[Severity: High]
Is it safe to establish this persistent mm_users reference? This ties the
BO's lifetime to the process's full memory footprint, leading to the leak
described above in amdxdna_gem_ubuf_obj_free().

> +
> +	ret = amdxdna_ubuf_hmm_register(abo, va_ent);
> +	if (ret)
> +		goto put_obj;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921184226.18941-1-lizhi.hou@amd.com?part=1