Linux io-uring development
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Oleg Sevostyanov <savant05@gmail.com>, io-uring@vger.kernel.org
Cc: Pavel Begunkov <asml.silence@gmail.com>
Subject: Re: [PATCH] io_uring/rsrc: use refcount_t for io_rsrc_node.refs
Date: Wed, 13 May 2026 17:55:27 -0600	[thread overview]
Message-ID: <d1098d70-30ca-487e-8e01-b8537eb23ade@kernel.dk> (raw)
In-Reply-To: <CAJv4CsvbaJd5GoHjYPzi3bgEO0fPT-3xj+UV7JMhqTyh2qr5tg@mail.gmail.com>

On 5/13/26 5:15 PM, Oleg Sevostyanov wrote:
> Hello,
> 
> This patch converts the refs field in struct io_rsrc_node from plain
> int to refcount_t.
> 
> Background
> ----------
> During a static analysis pass of io_uring/rsrc.{c,h} I examined all
> sites that touch io_rsrc_node.refs:
> 
>   - io_rsrc_node_alloc()     rsrc.c:147   initialises to 1
>   - io_buf_node_lookup()     rsrc.c:1117  refs++ under io_ring_submit_lock
>   - io_clone_buffers() x2    rsrc.c:1199  refs++ under uring_lock (lockdep_assert_held
>                              rsrc.c:1232  asserted on both ctx's)
>   - io_put_rsrc_node()       rsrc.h:107   --refs under uring_lock
> 
> All four sites are correctly guarded by ctx->uring_lock, so there is no
> present race or overflow risk.  This is a defence-in-depth change only.
> 
> Rationale
> ---------
> io_mapped_ubuf (defined in the same header, rsrc.h:40) already uses
> refcount_t for its own refs field.  Aligning io_rsrc_node to the same
> convention:

Because those can be shared across rings (cloning buffers), hence we
cannot rely on the ring lock for that.

>   1. Gives lockless overflow/underflow detection "for free" on kernels
>      built with REFCOUNT_FULL or on architectures that provide
>      REFCOUNT_ARCH_OPTIMIZED (x86 since 4.14).

It's certainly not "for free".

>   2. Makes it harder for a future patch that removes or relaxes locking
>      to silently introduce a refcount bug?the saturating behaviour of
>      refcount_t would catch wraps and emit a WARN_ONCE before a
>      use-after-free could occur.

You could just add a lockdep assert for that.

>   3. Self-documents the intent: the field is a reference counter, not an
>      arbitrary signed integer.

I mean, it's named ->refs, you'd think that'd make it clear enough.

> No functional change is intended.  I do not have a stable kernel build
> environment that includes the full io_uring tree, so I am unable to
> provide a Tested-by, but the patch compiles cleanly against the 6.8
> source tree (io_uring/ sparse checkout).

So in other words, you didn't even test this? And it's against an
ancient kernel?

None of that matters though, as that's a hard no on this patch.

-- 
Jens Axboe

      reply	other threads:[~2026-05-13 23:55 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-13 23:15 [PATCH] io_uring/rsrc: use refcount_t for io_rsrc_node.refs Oleg Sevostyanov
2026-05-13 23:55 ` Jens Axboe [this message]

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=d1098d70-30ca-487e-8e01-b8537eb23ade@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=asml.silence@gmail.com \
    --cc=io-uring@vger.kernel.org \
    --cc=savant05@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox