Hi Christian,
> Subject: Re: [PATCH v1 2/2] udmabuf: Add mlock accounting for
> udmabuf pages/folios
>
> 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.
The goal is to have the mlock accounting be tied to the exporters since they
are the ones that pin the pages/folios. As Sashiko identified, this patch
doesn't
quite do that but once fixed, both alloc and free accounting would be attributed
to the exporter. Would this idea not work?
Thanks,
Vivek
>
> 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);
