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 <[email protected]> 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/[email protected]?part=1
