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

Pre-existing issues:
- [High] Unrestricted userspace-controlled allocation size for `va_ent` can 
trigger the OOM killer.
--

commit 9b9a5c5cd1330d0aae0aea71c45dc018b59ee7e1
Author: Taimuraz Kaitmazov <[email protected]>

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

This patch fixes an issue where amdxdna_get_ubuf() could truncate a per-entry
page count derived from a 64-bit length into a 32-bit integer. It adds
validation to ensure the total page count fits within an integer, preventing
invalid memory accesses. It also explicitly rejects zero-length buffer
entries.

> diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.c 
> b/drivers/accel/amdxdna/amdxdna_ubuf.c
> index 0e0cd69cd1fbf..bf1e4dd7bbc30 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++) {

[Severity: High]
This is a pre-existing issue, but is it possible for num_entries to be a
massive value here?

Looking at amdxdna_gem_create_ubuf_object() in
drivers/accel/amdxdna/amdxdna_gem.c, it appears that va_tbl.num_entries
from userspace is passed directly to amdxdna_get_ubuf() without any bounds
checking.

Earlier in amdxdna_get_ubuf(), we allocate memory based on this value:

        va_ent = kvzalloc_objs(*va_ent, num_entries);

If an unprivileged user with a non-zero RLIMIT_MEMLOCK sets num_entries to a
huge value like U32_MAX, can this allocate an unrestricted amount of memory
and trigger the OOM killer before we even reach the bounds checks on nr_pages
added in this patch?

>               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;

[ ... ]

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to