> Subject: Re: [PATCH v2] dma-buf/udmabuf: Disable the size limit by
> default
>
> On 28.07.26 22:20, Kasireddy, Vivek wrote:
> > Hi Robert,
> >
> >> Subject: [PATCH v2] dma-buf/udmabuf: Disable the size limit by
> default
> >>
> >> As udmabuf increasingly enjoys popularity - being used in projects
> like
> >> libcamera, Gstreamer, Mesa, KWin and Weston - users more
> frequently
> >> encounter cases where the current default size limit of 64MB is too
> low.
> >> Examples include allocating video buffers at a 8K resolution - and
> even
> >> 4K
> >> is affected when using non-subsampled video formats and high bit
> >> depths.
> >>
> >> In its current form the size limit for individual buffers does not seem
> to
> >> provide any additional level of protection - such as limiting the
> amount
> >> of
> >> memory a process can pin - as the later can just allocate multiple
> >> buffers.
> >> If additional guardrails are desired, they would likely require some
> kind
> >> accounting not limited to individual buffers.
> >>
> >> Therefor let's disable the size limit by default by setting it to the
> >> maximal possible value, INT_MAX.
> >>
> >> Signed-off-by: Robert Mader <robert.mader(a)collabora.com>
> >>
> >> ---
> >>
> >> Changes in V2:
> >> - Use INT_MAX instead of 0 in order to not change behavior
> otherwise.
> >>
> >> See
> >> https://lore.kernel.org/dri-devel/20260711144814.8205-1-
> >> robert.mader(a)collabora.com/
> >> for a previous attempt to make the value configurable via kconfig -
> and
> >> in particular
> >> https://lore.kernel.org/dri-devel/6764ca6f-b4d8-4baa-9d27-
> >> 2ca867ac2d41(a)amd.com/
> >> for the suggestion and discussion to remove the default limit.
> >> ---
> >> drivers/dma-buf/udmabuf.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> >> index bced421c0d65..639e93704924 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;
> >> 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.");
> > Acked-by: Vivek Kasireddy <vivek.kasireddy(a)intel.com>
> >
> > If there are no further concerns/questions from anyone, I'll push it to
> > drm-misc-next soon.
>
> FTR., we already have a first duplicate :P
>
> https://lore.kernel.org/dri-devel/20260803144120.11524-1-
> xaver.hugl(a)kde.org/
Yeah, looks like this change seems to be desirable in many use-cases.
I have pushed it to drm-misc-next today.
Thanks,
Vivek
>
> >
> > Thanks,
> > Vivek
> >> struct udmabuf {
> >> pgoff_t pagecount;
> >> --
> >> 2.55.0
>
> --
> Robert Mader
> Consultant Software Developer
>
> Collabora Ltd.
> Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
> Registered in England & Wales, no. 5513718
On Fri, 31 Jul 2026 15:01:01 -0700 Bobby Eshleman wrote:
> Meanwhile, do we want drivers/net/hw/config to workaround by carrying
> something like SYNC_FILE or DMABUF_HEAPS, or wait and see about the
> udmabuf kconfig change?
Let's enable SYNC_FILE for now and pursue the UDMABUF change in
parallel. It will land in another tree if at all, so we'll have
to wait a while before it propagates to us.
On Wed, Jul 29, 2026 at 11:27 PM Baineng Shou <shoubaineng(a)gmail.com> wrote:
>
> Add a test case that verifies no file descriptor is leaked when
> DMA_HEAP_IOCTL_ALLOC succeeds internally but copy_to_user() fails
> to deliver the fd number back to userspace.
>
> The failure is triggered by placing the ioctl argument in a private
> anonymous page and flipping it to PROT_READ (via mprotect) between
> the kernel's copy_from_user() and copy_to_user() calls. With the
> buggy kernel the ioctl returns -EFAULT but leaves an extra open fd
> in the process's fd table; with the fixed kernel the fd count is
> unchanged.
>
> This serves as a regression test for:
> "dma-buf: dma-heap: don't publish fd before copy_to_user() succeeds"
>
> Suggested-by: Sumit Semwal <sumit.semwal(a)linaro.org>
> Signed-off-by: Baineng Shou <shoubaineng(a)gmail.com>
> ---
> .../selftests/dmabuf-heaps/dmabuf-heap.c | 115 +++++++++++++++++-
> 1 file changed, 114 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
> index fc9694fc4e89..bd58e5b06c8b 100644
> --- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
> +++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
> @@ -390,6 +390,118 @@ static void test_alloc_errors(char *heap_name)
> close(heap_fd);
> }
>
> +/*
> + * test_alloc_no_fd_leak_on_efault - verify no fd is leaked when
> + * copy_to_user() fails during DMA_HEAP_IOCTL_ALLOC.
> + *
> + * The bug: dma_buf_fd() called fd_install() before copy_to_user().
> + * If copy_to_user() then failed (e.g. via mprotect), the fd was
> + * silently installed in the fd table but never returned to userspace.
> + *
> + * The fix: reserve the fd with get_unused_fd_flags() first, attempt
> + * copy_to_user(), and only call fd_install() on success.
> + *
> + * We trigger the failure by placing the ioctl argument in a page,
> + * flipping it to PROT_READ between copy_from_user and copy_to_user,
> + * and counting open file descriptors before and after.
> + */
> +static void test_alloc_no_fd_leak_on_efault(char *heap_name)
> +{
> + int heap_fd = -1;
> + int fd_before, fd_after;
> + int ret;
> + long page_size;
> + struct dma_heap_allocation_data *req;
> +
> + ksft_print_msg("Testing no fd leak when copy_to_user() fails:\n");
> +
> + heap_fd = dmabuf_heap_open(heap_name);
> +
> + page_size = sysconf(_SC_PAGESIZE);
> +
> + /*
> + * Place the ioctl argument in its own private anonymous page so
> + * we can flip its protection independently.
> + */
> + req = mmap(NULL, page_size, PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> + if (req == MAP_FAILED) {
> + ksft_test_result_fail("mmap failed: %s\n", strerror(errno));
> + goto out;
> + }
> +
> + memset(req, 0, sizeof(*req));
> + req->len = page_size;
> + req->fd_flags = O_RDWR | O_CLOEXEC;
> +
> + /* Count open fds before the ioctl */
> + fd_before = 0;
> + {
> + DIR *d = opendir("/proc/self/fd");
> + struct dirent *de;
> +
> + if (!d) {
> + ksft_test_result_fail("opendir /proc/self/fd: %s\n",
> + strerror(errno));
> + munmap(req, page_size);
> + goto out;
> + }
> + while ((de = readdir(d)))
> + if (de->d_name[0] != '.')
> + fd_before++;
> + closedir(d);
> + /* subtract the fd opened by opendir itself */
But no actual subtraction?
> + }
> +
> + /*
> + * Make the page read-only: copy_from_user() in the kernel will
> + * still succeed (it already ran),
Huh? copy_from_user hasn't run yet. That happens inside the ioctl().
> but copy_to_user() that writes
> + * the fd number back will fault.
> + */
> + mprotect(req, page_size, PROT_READ);
> +
> + ret = ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, req);
> +
> + /* Re-allow writes so munmap can clean up */
> + mprotect(req, page_size, PROT_READ | PROT_WRITE);
> + munmap(req, page_size);
> +
> + if (ret != -1 || errno != EFAULT) {
This looks like you meant &&, but I think we should just fail if ret
!= -1. Either the mprotect is broken, or dma-heap didn't actually try
to copy_to_user.
> + /*
> + * If the ioctl didn't fail with EFAULT, either the kernel
> + * handled it differently or mprotect raced.
mprotect is synchronous, how could it race with anything here?
> Skip rather
> + * than giving a false pass/fail.
> + */
> + ksft_test_result_skip(
> + "ioctl did not return EFAULT (ret=%d errno=%d), skipping\n",
> + ret, errno);
> + goto out;
> + }
> +
> + /* Count open fds after the failed ioctl */
> + fd_after = 0;
> + {
> + DIR *d = opendir("/proc/self/fd");
> + struct dirent *de;
> +
> + if (!d) {
> + ksft_test_result_fail("opendir /proc/self/fd: %s\n",
> + strerror(errno));
> + goto out;
> + }
> + while ((de = readdir(d)))
> + if (de->d_name[0] != '.')
> + fd_after++;
> + closedir(d);
> + }
> +
> + ksft_test_result(fd_before == fd_after,
> + "no fd leak on EFAULT: before=%d after=%d\n",
This is for the failure case, so I don't think the "no" should be in the string.
> + fd_before, fd_after);
> +out:
> + close(heap_fd);
> +}
> +
> static int numer_of_heaps(void)
> {
> DIR *d = opendir(DEVPATH);
> @@ -420,7 +532,7 @@ int main(void)
> return KSFT_SKIP;
> }
>
> - ksft_set_plan(11 * numer_of_heaps());
> + ksft_set_plan(12 * numer_of_heaps());
>
> while ((dir = readdir(d))) {
> if (!strncmp(dir->d_name, ".", 2))
> @@ -435,6 +547,7 @@ int main(void)
> test_alloc_zeroed(dir->d_name, ONE_MEG);
> test_alloc_compat(dir->d_name);
> test_alloc_errors(dir->d_name);
> + test_alloc_no_fd_leak_on_efault(dir->d_name);
> }
> closedir(d);
>
> --
> 2.34.1
>
On Thu, 30 Jul 2026 14:39:37 -0700 Bobby Eshleman wrote:
> Poking around, it looks like 231.1.167.0 and above should support
> everything, so fw should be okay AFAICT.
>
> Looks like the config is missing CONFIG_NET_DEVMEM and CONFIG_UDMABUF?
Ah, damn, you're right. I grepped for DEVMEM and didn't look closely on
a hit. Turns out there's a non-NET DEVMEM, too.
Could you send a patch to add the missing config options to
tools/testing/selftests/drivers/net/hw/config ?
Can be separate or part of this series, doesn't matter.
> Sorry, took me a while... was certain it was a bug in my code.
>
> Not the failure here, but wondering if this was on ARM led to seeing
> that 16K hardcoded rx_page_size in run_rx_large_niov() may fail on ARM
> with 64K pages because it will fail the IS_ALIGN(16K, 64K) check...
Ah, good thought. We've been meaning to get an ARM64 server for NIPA
(our current server supplier is out). If it's not too hard could be nice
to guard against that. But also not a huge deal for HW tests if they
fail instead of skipping.