On Tue, Jul 28, 2026 at 10:29:14PM +0100, Pavel Begunkov wrote:
> Introduce a new iterator type for dmabuf maps. The map in an opaque
> object with internals and format specific to the subsystem / driver, and
> only it can use that subsystem / driver for issuing IO. The task of the
> middle layers is to pass the map / iterator further down, maybe doing
> basic splitting and length checking. The iterator can only be used by
> operations of the file the associated map was created for.
>
> Suggested-by: Keith Busch <kbusch(a)kernel.org>
> Signed-off-by: Pavel Begunkov <asml.silence(a)gmail.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch(a)lst.de>
I'm also pretty sure I reviewed this and vairous other patches
before..
The method name feels a bit convoluted, but given all the
previous discussions I don't care too strongly. I'll leave
the dma-buf side review to those who understand it.
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.
Thanks,
Vivek
>
> struct udmabuf {
> pgoff_t pagecount;
> --
> 2.55.0
Hello Baineng,
On Mon, 27 Jul 2026 at 08:56, 寿柏能 <shoubaineng(a)gmail.com> wrote:
>
> Ping. Any chance this can be picked up?
Thank you for the patch.
While discussing with T J, we realised that adding a selftest for this
would also be helpful. Could you please re-submit with a selftest
added?
>
> Baineng
Thanks and best regards,
Sumit.
>
> 寿柏能 <shoubaineng(a)gmail.com> 于2026年7月15日周三 10:04写道:
>>
>> Hi David
>>
>> The rollback here is the simplest possible case (put_unused_fd +
>> dma_buf_put), not the complex sockopt scenario you describe.
>>
>> Baineng
>>
>> David Laight <david.laight.linux(a)gmail.com> 于2026年7月14日周二 22:33写道:
>>>
>>> On Tue, 14 Jul 2026 21:38:07 +0800
>>> 寿柏能 <shoubaineng(a)gmail.com> wrote:
>>>
>>> > Hi David,
>>> >
>>> > Thanks for the feedback.
>>> >
>>> > The concern is not just about the EFAULT return — it's about the race
>>> > window between fd_install() and copy_to_user(). Once fd_install()
>>> > returns, the fd is immediately observable by other threads in the same
>>> > process (via /proc/self/fd, SCM_RIGHTS, etc.), even before
>>> > copy_to_user() has a chance to fail. The triggering condition is a
>>> > deliberate mprotect() flip, not a corrupted heap.
>>>
>>> That is what makes doing the close wrong.
>>> But that is a program aggressively trying to hit the timing window,
>>> not a normal program that has managed to pass an invalid pointer.
>>> The most likely reason for a real program passing an invalid pointer
>>> is a corrupted heap (assuming the stupid coding errors are fixed).
>>>
>>> It is really no different from the sockopt code that receives
>>> SCM_RIGHTS messages.
>>> In that case once you've removed the FILE from the socket (or similar)
>>> you really don't want to have to put it back because the write to the
>>> sockopt buffer or length field fails.
>>> The chance of correctly reverting the kernel state is small - and won't
>>> be tested.
>>>
>>> David
>>>
>>> >
>>> > The fix itself is small and follows the standard kernel idiom:
>>> > get_unused_fd_flags() reserves the fd without publishing it, so the
>>> > window between reservation and install is entirely under kernel control.
>>> >
>>> > Baineng
>>> >
>>> > David Laight <david.laight.linux(a)gmail.com> 于2026年7月14日周二 21:14写道:
>>> >
>>> > > On Tue, 14 Jul 2026 19:46:53 +0800
>>> > > Baineng Shou <shoubaineng(a)gmail.com> wrote:
>>> > >
>>> > > > DMA_HEAP_IOCTL_ALLOC allocates a dma-buf and installs an fd into the
>>> > > > caller's fd table via dma_buf_fd() -> fd_install() before
>>> > > > dma_heap_ioctl() copies the result back to userspace. If the trailing
>>> > > > copy_to_user() fails, userspace never learns the fd number, but the
>>> > > > fd (and the underlying dma-buf reference) are already visible to
>>> > > > other threads in the same process and are leaked for the lifetime of
>>> > > > the process.
>>> > > >
>>> > > > The obvious "close it on the failure path" fix is unsafe: once
>>> > > > fd_install() has run, another thread can already dup() the fd, send
>>> > > > it via SCM_RIGHTS, or close() it and let its number be reused, so a
>>> > > > subsequent close_fd() from the ioctl path can operate on an unrelated
>>> > > > file. This was pointed out by Christian König on v1 [1].
>>> > > ...
>>> > >
>>> > > My 2c:
>>> > >
>>> > > The other option is just to leave it as a 'problem for user space'.
>>> > > No reasonable program is going to handle the EFAULT return by doing
>>> > > anything other than exiting.
>>> > > Even getting an EFAULT is really an indication that the application
>>> > > is already in a real mess - most likely with a badly corrupted heap.
>>> > >
>>> > > Anything else leaves error recovery code in the kernel that is pretty
>>> > > much never executed and open to a variety of bugs.
>>> > > While the recovery here is probably ok, there are some sockopt calls
>>> > > where it is all more complicated.
>>> > >
>>> > > David
>>> > >
>>>