Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Unprivileged users can exhaust system memory by pinning an unbounded amount of pages, bypassing RLIMIT_MEMLOCK limits. - [High] On 32-bit systems, an integer overflow in `udmabuf_create` during the accumulation of `pgcnt` allows a local attacker to bypass the `size_limit_mb` check and cause a kernel heap buffer overflow. -- --- Patch [1]: [PATCH] udmabuf: vastly increase the size limit per dmabuf --- commit 35feed24a80e2f326fb9ba08e55d7ef8e97bae25 Author: Xaver Hugl <[email protected]> udmabuf: vastly increase the size limit per dmabuf This commit increases the default size limit per dmabuf from 64MB to 1024MB. This accommodates larger buffers used by high-resolution displays when scaling is applied. > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index bced421c0d658..be2bcda0fb0f3 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -20,9 +20,9 @@ static int list_limit = 1024; > module_param(list_limit, int, 0644); > MODULE_PARM_DESC(list_limit, "udmabuf_create_list->count limit. Default is > 1024."); > > -static int size_limit_mb = 64; > +static int size_limit_mb = 1024; [Severity: High] This is a pre-existing issue, but does increasing the default size limit to 1024MB make it easier for unprivileged users to exhaust system memory? In drivers/dma-buf/udmabuf.c:udmabuf_create(), memory is pinned using memfd_pin_folios() but it doesn't appear to call account_locked_vm() or check against RLIMIT_MEMLOCK: drivers/dma-buf/udmabuf.c:udmabuf_create() { ... ret = udmabuf_pin_folios(ubuf, memfd, list[i].offset, list[i].size, folios); ... } Without a hard limit on the total number of dmabufs a user can create, an unprivileged user with access to /dev/udmabuf might be able to repeatedly allocate dmabufs up to the size_limit_mb per file descriptor and pin system memory until the tmpfs limit or physical RAM is completely exhausted. [Severity: High] This isn't a bug introduced by this patch, but does the larger size limit make it easier to hit an integer overflow on 32-bit systems? In drivers/dma-buf/udmabuf.c:udmabuf_create(), pgcnt is accumulated using a 32-bit pgoff_t on 32-bit systems: drivers/dma-buf/udmabuf.c:udmabuf_create() { ... subpgcnt = list[i].size >> PAGE_SHIFT; pgcnt += subpgcnt; if (pgcnt > pglimit) goto err_noinit; ... } By supplying up to list_limit (default 1024) items, an attacker could spread 2^32 pages across the items such that pgcnt wraps around to a small value, bypassing the pglimit check. init_udmabuf() then allocates an undersized ubuf->pages array. Later, when udmabuf_pin_folios() successfully pins the large items, it iterates over the actual requested size and can write out of bounds into the small heap array: drivers/dma-buf/udmabuf.c:udmabuf_pin_folios() { ... for (; subpgoff < fsize; subpgoff += PAGE_SIZE) { ubuf->pages[upgcnt] = folio_page(folios[cur_folio], subpgoff >> PAGE_SHIFT); ++upgcnt; } ... } Triggering this out-of-bounds write requires memfd_pin_folios() to successfully pin the largest item (minimum ~16.7 GB spread across 1024 items). Thus, it relies on the 32-bit system having enough physical RAM available (e.g. via PAE) to not fail safely with -ENOMEM before reaching the vulnerable write. > module_param(size_limit_mb, int, 0644); > -MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in megabytes. Default > is 64."); > +MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in megabytes. Default > is 1024."); > > struct udmabuf { > pgoff_t pagecount; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
