Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Removing the dmabuf size limit (by setting `size_limit_mb` to `INT_MAX`) allows an unprivileged user to request a massive buffer size (up to ~2.2 TB). This forces the kernel to attempt massive unbounded vmalloc allocations for the internal tracking arrays, rapidly exhausting system memory and triggering the OOM killer. Pre-existing issues: - [High] Setting `size_limit_mb` to `INT_MAX` allows unprivileged users to create udmabufs of 4GB or larger. On 32-bit architectures, this triggers an integer overflow when converting the page count back to bytes (`ubuf->pagecount << PAGE_SHIFT`), resulting in a size of 0 or a severely truncated value. -- --- Patch [1]: [PATCH v2] dma-buf/udmabuf: Disable the size limit by default --- commit f1c9db60e57729e9743c42b43759bab6d0eafb5d Author: Robert Mader <[email protected]> dma-buf/udmabuf: Disable the size limit by default This commit disables the default 64MB size limit for udmabuf by setting it to INT_MAX. It aims to support users who need larger buffers for high resolution video formats. The change assumes the per-buffer size limit does not provide sufficient protection on its own. Link: https://lore.kernel.org/dri-devel/[email protected]/ Link: https://lore.kernel.org/dri-devel/[email protected]/ > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index bced421c0d658..639e937049242 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 = INT_MAX; [Severity: High] Does this change allow an unprivileged user to exhaust kernel memory? Because the size limit is effectively removed, an unprivileged user can request an extremely large buffer size via /dev/udmabuf. Looking at udmabuf_create() and init_udmabuf(), the internal tracking arrays are allocated based on this requested size: drivers/dma-buf/udmabuf.c:udmabuf_create() { ... folios = kvmalloc_array(max_nr_folios, sizeof(*folios), GFP_KERNEL); ... } drivers/dma-buf/udmabuf.c:init_udmabuf() { ... ubuf->pages = kvmalloc_objs(*ubuf->pages, pgcnt); ... } Since these allocations use GFP_KERNEL without __GFP_ACCOUNT, they bypass memory cgroups. Could a large request trigger unbounded vmalloc allocations that consume gigabytes of memory per ioctl call, leading to an OOM condition? [Severity: High] This is a pre-existing issue, but does removing the size limit expose a page count overflow on 32-bit architectures? When a user requests a size of 4GB or larger, the pagecount exceeds 1048576. In export_udmabuf() and get_sg_table(), the size is calculated by shifting the page count: drivers/dma-buf/udmabuf.c:export_udmabuf() { ... exp_info.size = ubuf->pagecount << PAGE_SHIFT; ... } drivers/dma-buf/udmabuf.c:get_sg_table() { ... ret = sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->pagecount, 0, ubuf->pagecount << PAGE_SHIFT, GFP_KERNEL); ... } On 32-bit architectures, this 32-bit arithmetic wraps around to 0. Can this result in a truncated dma-buf size or an invalid SG table with 0-length elements, potentially causing a panic in IOMMU drivers? > 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 INT_MAX."); > > struct udmabuf { > pgoff_t pagecount; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
