Re: [PATCH v6 11/13] io_uring/rsrc: add uncloneable regbuf flag

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] io_uring: incorrect error code -ENOMEM for uncloneable buffers
--

commit 09226c4686b3c73a879519b5ef4315732d82b7f5
Author: Pavel Begunkov <asml.silence@gmail.com>

io_uring/rsrc: add uncloneable regbuf flag

It's hard to implement cloning if the internal structure needs to be
mutable and/or relies on other ring resources. In preparation to such
buffer types, add a flag indicating that the buffer can't be cloned.

> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> index 83a7ad9f5bc78..0b526b094dc9e 100644
> --- a/io_uring/rsrc.c
> +++ b/io_uring/rsrc.c
> @@ -1434,6 +1434,11 @@ static int io_clone_buffers(struct io_ring_ctx *ctx, struct io_ring_ctx *src_ctx
>  		if (!src_node) {
>  			dst_node = NULL;
>  		} else {
> +			if (src_node->buf->flags & IO_REGBUF_F_UNCLONEABLE) {
> +				io_rsrc_data_free(ctx, &data);
> +				return -ENOMEM;

[Severity: Medium]
Is -ENOMEM the correct error code to return from io_clone_buffers() for this
case? Returning -ENOMEM for an unsupported operation (cloning an explicitly
uncloneable buffer) might be misleading, as it usually signifies a transient
out-of-memory condition.

If a user registers an uncloneable buffer and subsequently attempts to clone
the buffer table via the IORING_REGISTER_CLONE_BUFFERS operation, robust
userspace applications encountering -ENOMEM might enter a back-off and retry
loop. This could potentially lead to infinite stalls rather than aborting the
unsupported operation. Would an error code like -EOPNOTSUPP or -EINVAL be
more appropriate to indicate that the buffer type simply doesn't support
cloning?

> +			}
> +
>  			dst_node = io_rsrc_node_alloc(ctx, IORING_RSRC_BUFFER);
>  			if (!dst_node) {
>  				io_rsrc_data_free(ctx, &data);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789997898.git.asml.silence@gmail.com?part=11