Re: [PATCH] accel/amdxdna: Bound the page count of a user supplied buffer

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Unhandled zero-size user buffer bypasses constraints and leads to a fatal kernel panic due to ZERO_SIZE_PTR dereference.
--

commit 0ce5a2a6bc3b9a95a5dd641e22f058018b729ead
Author: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>

accel/amdxdna: Bound the page count of a user supplied buffer

amdxdna_get_ubuf() puts a per-entry page count derived from a __u64
va_ent[i].len into a u32, then passes it to pin_user_pages_fast().
This patch rejects a total that does not fit in an int to prevent
truncation and unbound memory operations.

> diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.c b/drivers/accel/amdxdna/amdxdna_ubuf.c
> index 0e0cd69cd1fbf..da8e32566ae0a 100644
> --- a/drivers/accel/amdxdna/amdxdna_ubuf.c
> +++ b/drivers/accel/amdxdna/amdxdna_ubuf.c
> @@ -125,6 +125,12 @@ struct dma_buf *amdxdna_get_ubuf(struct drm_device *dev,
>  	}
>  
>  	ubuf->nr_pages = exp_info.size >> PAGE_SHIFT;
> +	if (ubuf->nr_pages > INT_MAX) {
> +		XDNA_ERR(xdna, "Too many pages %lld", ubuf->nr_pages);
> +		ret = -EINVAL;
> +		goto free_ent;
> +	}
> +

[Severity: Critical]
This isn't a bug introduced by this patch, but is it possible for
ubuf->nr_pages to be 0 here, leading to a kernel panic later?

If a user calls the AMDXDNA_DRM_CREATE_BO ioctl with a virtual address
table containing at least one entry but with lengths set to 0,
exp_info.size becomes 0 and ubuf->nr_pages becomes 0. This bypasses the
bounds check above.

Then kvmalloc_objs() allocates ubuf->pages with size 0, which returns
ZERO_SIZE_PTR (0x10) instead of NULL. Later, pin_user_pages_fast() handles
npages = 0 by returning 0, allowing the zero-length dma-buf to be
exported.

When the buffer is eventually mapped in amdxdna_ubuf_map() in
amdxdna_ubuf.c:

ret = sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->nr_pages, 0,
                                ubuf->nr_pages << PAGE_SHIFT, GFP_KERNEL);

sg_alloc_table_from_pages() delegates to sg_alloc_append_table_from_pages(),
which computes chunks = 1 and evaluates pages[0] in its first iteration
since the nested loop bounds check is skipped when n_pages == 0.

Dereferencing ZERO_SIZE_PTR (0x10) will cause a fatal kernel page fault.

Could we add a check for ubuf->nr_pages == 0 here as well?

>  	lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>  	new_pinned = atomic64_add_return(ubuf->nr_pages, &ubuf->mm->pinned_vm);
>  	if (new_pinned > lock_limit && !capable(CAP_IPC_LOCK)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826195736.358579-1-taimuraz@kaitmazov.com?part=1