Re: [PATCH v1 2/2] udmabuf: Add mlock accounting for udmabuf pages/folios

[Robert Mader]

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.

Best regards

Robert


Signed-off-by: Vivek Kasireddy <vivek.kasire...@intel.com>
Cc: Gerd Hoffmann <kra...@redhat.com>
Cc: "Christian König" <christian.koe...@amd.com>
Cc: Robert Mader <robert.ma...@collabora.com>
Cc: Xaver Hugl <xaver.h...@kde.org>
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);