Hi Robert,
> Subject: Re: [PATCH v1 2/2] udmabuf: Add mlock accounting for
> udmabuf pages/folios
>
> Hi, thanks for the patch!
>
> On 31.08.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.
>
> I think this description is somewhat misleading: before commit
> 44e9eb5a7621 the same was already possible - it just needed multiple
> buffers/allocations. That's why that patch was accepted in the first
> place, without first landing a patch like this one (adding mlock
> accounting). I.e. the described problem is not new and just became
> easier to trigger.
Ok, I'll update the description to say that commit 44e9eb5a7621 just
exposed an already existing problem and made it easier to trigger it.
Thanks,
Vivek
>
> Best regards
>
> Robert
>
> >
> > 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);
