On 8/31/26 08:00, Vivek Kasireddy wrote:
> Since udmabuf allows users to pin pages in memory, which is a vital
> resource, we need to ensure that unprivileged users cannot abuse
> this feature to exhaust system memory. To address this, introduce
> mlock accounting for udmabuf pages/folios. This change ensures that
> the number of pages/folios pinned by udmabuf is tracked and limited
> based on the user's mlock limits.
>
> If an unprivileged user/process exceeds their mlock limit, no pages
> are pinned and EPERM is returned to the user. Note that, privileged
> users/processes (CAP_IPC_LOCK capable) are exempt from this limit,
> allowing them to pin pages without any restriction.
>
> Earlier, there was a 64 MB default limit on the size of udmabuf
> allocations, but it was removed in commit 44e9eb5a7621
> ("dma-buf/udmabuf: Disable the size limit by default") which left
> open the possibility that unpriviledged users could pin unbounded
> amount of memory. This change makes the limit dynamic, by invoking
> user_shm_lock() to check the mlock limit for unprivileged users.
That makes no sense at all and would create a massive problems.
The mlock limit is per process, but udmabuf is represented by a file descriptor
which can move between processes.
So this here can be trivially abused to account the allocation to one process
while accounting the free to another process and overflowing the mlock
housekeeping.
What could potentially work is to grab a reference to the cred structure of the
allocating task, but that is also not the nicest thing to do.
Regards,
Christian.
>
> Signed-off-by: Vivek Kasireddy <[email protected]>
> Cc: Gerd Hoffmann <[email protected]>
> Cc: "Christian König" <[email protected]>
> Cc: Robert Mader <[email protected]>
> Cc: Xaver Hugl <[email protected]>
> Fixes: 44e9eb5a7621 ("dma-buf/udmabuf: Disable the size limit by default")
> ---
> drivers/dma-buf/udmabuf.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index 7c4339130eff..e78947f01656 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -203,9 +203,10 @@ static __always_inline int init_udmabuf(struct udmabuf
> *ubuf, pgoff_t pgcnt)
> return 0;
> }
>
> -static __always_inline void deinit_udmabuf(struct udmabuf *ubuf)
> +static __always_inline void deinit_udmabuf(struct udmabuf *ubuf, pgoff_t
> pgcnt)
> {
> unpin_all_folios(ubuf);
> + user_shm_unlock(pgcnt << PAGE_SHIFT, current_ucounts());
> kvfree(ubuf->pages);
> }
>
> @@ -217,7 +218,7 @@ static void release_udmabuf(struct dma_buf *buf)
> if (ubuf->sg)
> put_sg_table(dev, ubuf->sg, ubuf->sg_dir);
>
> - deinit_udmabuf(ubuf);
> + deinit_udmabuf(ubuf, ubuf->pagecount);
> kfree(ubuf);
> }
>
> @@ -385,6 +386,15 @@ static long udmabuf_create(struct miscdevice *device,
> if (!pgcnt)
> goto err_noinit;
>
> + if (!user_shm_lock(pgcnt << PAGE_SHIFT, current_ucounts())) {
> + pr_warn_once("%s: User not permitted to pin more than %lu KB "
> + "of memory, either increase ulimits -l, or "
> + "obtain CAP_IPC_LOCK capability\n", current->comm,
> + rlimit(RLIMIT_MEMLOCK) >> 10);
> + ret = -EPERM;
> + goto err_noinit;
> + }
> +
> ret = init_udmabuf(ubuf, pgcnt);
> if (ret)
> goto err;
> @@ -441,7 +451,7 @@ static long udmabuf_create(struct miscdevice *device,
> return ret;
>
> err:
> - deinit_udmabuf(ubuf);
> + deinit_udmabuf(ubuf, pgcnt);
> err_noinit:
> kfree(ubuf);
> kvfree(folios);
