Re: [PATCH] udmabuf: vastly increase the size limit per dmabuf

[sashiko-bot@kernel.org]

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 <xaver.hugl@kde.org>
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/20260803144120.11524-1-xaver.hugl@kde.org?part=1