netdev - Re: [PATCH v2 3/5] io_uring/zcrx: share an ifq between rings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f1fa5543-c637-435d-a189-5d942b1c7ebc@kernel.dk>
Date: Sat, 25 Oct 2025 17:41:15 -0600
From: Jens Axboe <axboe@...nel.dk>
To: David Wei <dw@...idwei.uk>, io-uring@...r.kernel.org,
 netdev@...r.kernel.org
Cc: Pavel Begunkov <asml.silence@...il.com>
Subject: Re: [PATCH v2 3/5] io_uring/zcrx: share an ifq between rings

On 10/25/25 1:15 PM, David Wei wrote:
> @@ -541,6 +541,74 @@ struct io_mapped_region *io_zcrx_get_region(struct io_ring_ctx *ctx,
>  	return ifq ? &ifq->region : NULL;
>  }
>  
> +static int io_proxy_zcrx_ifq(struct io_ring_ctx *ctx,
> +			     struct io_uring_zcrx_ifq_reg __user *arg,
> +			     struct io_uring_zcrx_ifq_reg *reg)
> +{
> +	struct io_zcrx_ifq *ifq, *src_ifq;
> +	struct io_ring_ctx *src_ctx;
> +	struct file *file;
> +	int src_fd, ret;
> +	u32 src_id, id;
> +
> +	src_fd = reg->if_idx;
> +	src_id = reg->if_rxq;
> +
> +	file = io_uring_register_get_file(src_fd, false);
> +	if (IS_ERR(file))
> +		return PTR_ERR(file);
> +
> +	src_ctx = file->private_data;
> +	if (src_ctx == ctx)
> +		return -EBADFD;
> +
> +	mutex_unlock(&ctx->uring_lock);
> +	io_lock_two_rings(ctx, src_ctx);
> +
> +	ret = -EINVAL;
> +	src_ifq = xa_load(&src_ctx->zcrx_ctxs, src_id);
> +	if (!src_ifq || src_ifq->proxy)
> +		goto err_unlock;
> +
> +	percpu_ref_get(&src_ctx->refs);
> +	refcount_inc(&src_ifq->refs);
> +
> +	ifq = kzalloc(sizeof(*ifq), GFP_KERNEL);

This still needs a:

	if (!ifq)
		handle error

addition, like mentioned for v1. Would probably make sense to just
assume that everything is honky dory and allocate it upfront/early, and
just kill it in the error path. Would probably help remove one of the
goto labels.

> +	ifq->proxy = src_ifq;

For this, since the ifq is shared and reference counted, why don't they
just point at the same memory here? Would avoid having this ->proxy
thing and just skipping to that in other spots where the actual
io_zcrx_ifq is required?

-- 
Jens Axboe