Re: [PATCH v2] dma-buf/udmabuf: Disable the size limit by default

[Robert Mader <robert.mader@collabora.com>]

Responding to the report just for the record:

On 22.07.26 13:27, sashiko-bot@kernel.org wrote:
> 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 <robert.mader@collabora.com>
>
> 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/20260711144814.8205-1-robert.mader@collabora.com/
> Link: https://lore.kernel.org/dri-devel/6764ca6f-b4d8-4baa-9d27-2ca867ac2d41@amd.com/
>
>> 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?
As mentioned in the commit message and as discussed in the mentioned 
links, this is a pre-existing issue as users could just allocate 
multiple buffer, having the same effect as allocating a single big one.
> [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:
The udmabuf interface works on pre-allocated system memory. On a 32-bit 
architecture users can't allocate more than 4GB of memory - so the 
described request should be impossible.
>
> 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;

-- 
Robert Mader
Consultant Software Developer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718