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

[Lizhi Hou <lizhi.hou@amd.com>]

On 8/26/26 12:57, Taimuraz Kaitmazov wrote:
> 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(), whose
> nr_pages is an int. An entry of 2^44 bytes truncates npages to zero, so
> nothing is pinned, the ret != npages test still passes, and ubuf->pages
> keeps whatever kvmalloc_objs() returned while ubuf->nr_pages describes the
> untruncated count. amdxdna_ubuf_release() then walks all of it. Reaching
> that needs CAP_IPC_LOCK and a multi-gigabyte kvmalloc() to succeed.
>
> Reject a total that does not fit in an int. The lengths are page aligned
> and summed with check_add_overflow(), so the total is at least as large as
> any one entry and bounds the pin call, the offset accumulator and
> sg_alloc_table_from_pages().
>
> Fixes: bd72d4acda10 ("accel/amdxdna: Support user space allocated buffer")
> Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
> ---
>   drivers/accel/amdxdna/amdxdna_ubuf.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.c b/drivers/accel/amdxdna/amdxdna_ubuf.c
> index 0e0cd69cd1fb..da8e32566ae0 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);

XDNA_DBG(.... %llu", )

Could you also add the boundary check sashiko suggested? Maybe something 
like:

         for (i = 0, exp_info.size = 0; i < num_entries; i++) {
                 if (!IS_ALIGNED(va_ent[i].vaddr, PAGE_SIZE) ||
-                   !IS_ALIGNED(va_ent[i].len, PAGE_SIZE)) {
-                       XDNA_ERR(xdna, "Invalid address or len %llx, %llx",
+                   !IS_ALIGNED(va_ent[i].len, PAGE_SIZE) ||
+                   !va_ent[i].len) {
+                       XDNA_DBG(xdna, "Invalid address or len %llx, %llx",
                                  va_ent[i].vaddr, va_ent[i].len);

Thanks,

Lizhi


> +		ret = -EINVAL;
> +		goto free_ent;
> +	}
> +
>   	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)) {