From: Pavel Begunkov <asml.silence@gmail.com>
To: Woraphat Khiaodaeng <worapat.kd2@gmail.com>,
Jens Axboe <axboe@kernel.dk>
Cc: David Wei <dw@davidwei.uk>,
io-uring@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] io_uring/zcrx: don't clear master_ctx from the import path
Date: Thu, 30 Jul 2026 20:16:07 +0100 [thread overview]
Message-ID: <7cdc0130-6dca-4e18-8f0e-1e7221fb9853@gmail.com> (raw)
In-Reply-To: <20260730162741.1125-1-worapat.kd2@gmail.com>
On 7/30/26 17:27, Woraphat Khiaodaeng wrote:
> import_zcrx() attaches an existing ifq to another ring. It never calls
> zcrx_set_ring_ctx() and so never takes the ->master_ctx reference, but
> its error path still passes @ctx to zcrx_unregister(), which clears
> ->master_ctx and drops its percpu_ref whenever ifq->master_ctx == ctx.
>
> That condition is reachable. A ring that registers an ifq with a
> non-zero event type_mask gets ->master_ctx pointed at itself, and
> nothing stops it from exporting that ifq with ZCRX_CTRL_EXPORT and
> importing the resulting fd back into the same ring. Failing the import
> after the refcount bumps -- an argument page mapped PROT_READ makes the
> copy_to_user() in import_zcrx() return -EFAULT -- then clears the
> ->master_ctx owned by the original registration, which is still live.
>
> Refcounts stay balanced and nothing is freed early, so there is no
> splat. The ring silently stops receiving ZCRX_EVENT_ALLOC_FAIL and
> ZCRX_EVENT_COPY: zcrx_send_notif() returns early on a NULL
> ->master_ctx, and ->master_ctx is only ever set on a freshly allocated
> ifq, so it cannot be restored without tearing the ring down.
Yep, it effectively disables event CQEs.
Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
...> diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
> index 76b9b0d54af9e..fea2b27272ec2 100644
> --- a/io_uring/zcrx.c
> +++ b/io_uring/zcrx.c
> @@ -808,7 +808,8 @@ static int import_zcrx(struct io_ring_ctx *ctx,
> scoped_guard(mutex, &ctx->mmap_lock)
> xa_erase(&ctx->zcrx_ctxs, id);
> err:
> - zcrx_unregister(ifq, ctx);
> + /* the import path never took the ->master_ctx ref, don't drop it */
nit: I'd say "... it never set ->master_ctx ..." as it's about having
a ctx and not references, but it's not worth of respinning.
> + zcrx_unregister(ifq, NULL);
> return ret;
> }
--
Pavel Begunkov
next prev parent reply other threads:[~2026-07-30 19:16 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-30 16:27 [PATCH] io_uring/zcrx: don't clear master_ctx from the import path Woraphat Khiaodaeng
2026-07-30 19:16 ` Pavel Begunkov [this message]
2026-07-30 19:19 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7cdc0130-6dca-4e18-8f0e-1e7221fb9853@gmail.com \
--to=asml.silence@gmail.com \
--cc=axboe@kernel.dk \
--cc=dw@davidwei.uk \
--cc=io-uring@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=worapat.kd2@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.