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

[Lizhi Hou]

On 8/26/26 14:28, 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().
>
> Reject a zero length entry as well: it contributes nothing to the mapping
> and a table of them leaves nr_pages at zero.
>
> Fixes: bd72d4acda10 ("accel/amdxdna: Support user space allocated buffer")
> Signed-off-by: Taimuraz Kaitmazov <taimuraz@xxxxxxxxxxxxx>
> ---
> v2:
> - XDNA_DBG and %llu, per your comment.
> - Reject a zero length entry in the validation loop, and lower that log to
>    XDNA_DBG too, as you suggested.
>
>   drivers/accel/amdxdna/amdxdna_ubuf.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.c b/drivers/accel/amdxdna/amdxdna_ubuf.c
> index 0e0cd69cd1fb..bf1e4dd7bbc3 100644
> --- a/drivers/accel/amdxdna/amdxdna_ubuf.c
> +++ b/drivers/accel/amdxdna/amdxdna_ubuf.c
> @@ -111,8 +111,9 @@ struct dma_buf *amdxdna_get_ubuf(struct drm_device *dev,
>   
>   	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);
>   			ret = -EINVAL;
>   			goto free_ent;
> @@ -125,6 +126,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_DBG(xdna, "Too many pages %llu", ubuf->nr_pages);
> +		ret = -EINVAL;
> +		goto free_ent;
> +	}
> +
Reviewed-by: Lizhi Hou <lizhi.hou@xxxxxxx>
>   	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)) {