Re: [PATCH] io_uring: do not charge user provided SQ/CQ rings to RLIMIT_MEMLOCK

[Pavel Begunkov <asml.silence@gmail.com>]

On 10/8/26 18:06, Hengyu Liang wrote:
> Commit 8078486e1d53 ("io_uring: use region api for SQ") and commit
> 81a4058e0cd0 ("io_uring: use region api for CQ") made io_uring_setup()
> create the rings with io_create_region().
> 
> However, io_create_region() charges user provided memory to
> RLIMIT_MEMLOCK, and the rings of an IORING_SETUP_NO_MMAP ring were not
> charged before those commits. As of now, a user without CAP_IPC_LOCK
> gets ENOMEM from io_uring_queue_init_mem() when their rings exceed the
> limit, which is 8 MiB by default. PostgreSQL 18 creates its rings with
> this function [1].

That patch you mentioned 26bfa89e25f4 ("io_uring: place ring SQ/CQ
arrays under memcg memory limits") has always been a delayed time bomb,
though memlock is quite a nasty limit. Makes me wonder if there is a
way to migrate it to cgroups completely.


...> The issue can be reproduced with a simple liburing program, run as an
> unprivileged user:
> 
> [2] https://lore.kernel.org/io-uring/b5a33433-b0b9-4231-9998-23e2a2202091@kernel.dk/
> 
>   io_uring/io_uring.c |  8 ++++----
>   io_uring/kbuf.c     |  2 +-
>   io_uring/memmap.c   |  7 ++++---
>   io_uring/memmap.h   |  2 +-
>   io_uring/register.c | 10 +++++-----
>   io_uring/zcrx.c     |  2 +-
>   6 files changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index c2ce83c7c1f1..2a16369894d4 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -2070,8 +2070,8 @@ int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr)
>   
>   static void io_rings_free(struct io_ring_ctx *ctx)
>   {
> -	io_free_region(ctx->user, &ctx->sq_region);
> -	io_free_region(ctx->user, &ctx->ring_region);

1. It's not perfect to make all these changes for a fix because of
backporting. Let's simplify it, add a wrapper and use the "__" version
only where needed.

__io_create_region(ctx, bool account, ...) {
	if (!ctx->user)
		account = false;
	...
}
io_create_region(ctx, ...) {
	return __io_create_region(ctx, true, ...);
}

2. The need to match alloc and free arguments has a high chance to
eventually blow up. It'd be better to turn it into a flag.

__io_create_region() {
	if (account) {
		...
		mr->flags |= IO_REGION_F_ACCOUNTED;
	}
}

io_free_region() {
	if ((mr->flags & IO_REGION_F_ACCOUNTED)) {
		WARN_ON_ONCE(!user);
		unaccount(user);
	}
}

-- 
Pavel Begunkov