All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: "Onur Özkan" <work@onurozkan.dev>, boqun@kernel.org
Cc: rcu@vger.kernel.org, rust-for-linux@vger.kernel.org,
	 linux-kernel@vger.kernel.org, ojeda@kernel.org,
	gary@garyguo.net,  bjorn3_gh@protonmail.com, lossin@kernel.org,
	a.hindborg@kernel.org,  tmgross@umich.edu, dakr@kernel.org,
	peterz@infradead.org,  fujita.tomonori@gmail.com,
	tamird@kernel.org, jiangshanlai@gmail.com,  paulmck@kernel.org,
	josh@joshtriplett.org, rostedt@goodmis.org,
	 mathieu.desnoyers@efficios.com
Subject: Re: [PATCH v10 4/5] rust: sync: add SRCU abstraction
Date: Mon, 6 Jul 2026 11:55:18 +0000	[thread overview]
Message-ID: <akuXpnwb_y4R7ekV@google.com> (raw)
In-Reply-To: <20260613065348.96750-5-work@onurozkan.dev>

On Sat, Jun 13, 2026 at 09:40:10AM +0300, Onur Özkan wrote:
> +#[pinned_drop]
> +impl PinnedDrop for Srcu {
> +    fn drop(self: Pin<&mut Self>) {
> +        let ptr = self.inner.get();
> +
> +        if crate::warn_on!(
> +            // SAFETY: By the type invariants, `self` contains a valid and pinned `struct srcu_struct`
> +            // and `srcu_readers_active()` only checks the active reader count.
> +            unsafe { bindings::srcu_readers_active(ptr) }
> +        ) {
> +            // `cleanup_srcu_struct()` may return early if there are still active readers.
> +            // This should only happen if a guard was leaked with `mem::forget`, which is
> +            // "WRONG" code and may cause a UAF because Rust will free the `srcu_struct`
> +            // while it is still referenced from the C side (e.g. by `call_srcu()` callbacks).
> +            //
> +            // Another consequence of leaking guards is that `call_srcu()` callbacks will
> +            // never run because the grace period can never complete due to permanently
> +            // active readers (i.e. leaked guards).
> +            //
> +            // If this ever happens, that means the guard was leaked by mistake and the
> +            // caller must fix the bug. Sleeping here is intentional and less harmful
> +            // than risking a UAF.
> +            //
> +            // SAFETY: By the type invariants, `self` contains a valid and pinned
> +            // `struct srcu_struct`.
> +            unsafe { bindings::synchronize_srcu(ptr) };
> +        }
> +
> +        // Ensure all SRCU callbacks have been finished before freeing.
> +        // SAFETY: By the type invariants, `self` contains a valid and pinned `struct srcu_struct`.
> +        unsafe { bindings::srcu_barrier(ptr) };

Hmm. It's not entirely clear to me that synchronize_srcu() is needed
here. If there are calls to srcu_read_lock() that do not have a matching
unlock due to use of mem::forget(), then either there is a pending
call_srcu() callback in the queue, in which case srcu_barrier() already
sleeps forever, or there are no such callbacks in which case I don't
think this actually leads to UAF.

Thoughts?

Alice

  reply	other threads:[~2026-07-06 11:55 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-13  6:40 [PATCH v10 0/5] rust: add SRCU abstraction Onur Özkan
2026-06-13  6:40 ` [PATCH v10 1/5] srcu: make init_srcu_struct() consistently wrap __init_srcu_struct() Onur Özkan
2026-07-10 13:38   ` Gary Guo
2026-06-13  6:40 ` [PATCH v10 2/5] rust: helpers: add SRCU helpers Onur Özkan
2026-07-10 13:39   ` Gary Guo
2026-06-13  6:40 ` [PATCH v10 3/5] srcu: expose srcu_readers_active() Onur Özkan
2026-06-13  6:40 ` [PATCH v10 4/5] rust: sync: add SRCU abstraction Onur Özkan
2026-07-06 11:55   ` Alice Ryhl [this message]
2026-07-06 12:39     ` Gary Guo
2026-07-06 13:01       ` Onur Özkan
2026-07-10 13:42   ` Gary Guo
2026-06-13  6:40 ` [PATCH v10 5/5] MAINTAINERS: add Rust SRCU files to SRCU entry Onur Özkan
2026-07-02 13:53 ` [PATCH v10 0/5] rust: add SRCU abstraction Onur Özkan
2026-07-10 12:38 ` Alice Ryhl
2026-07-10 14:18 ` Boqun Feng
2026-07-10 18:07   ` Paul E. McKenney

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=akuXpnwb_y4R7ekV@google.com \
    --to=aliceryhl@google.com \
    --cc=a.hindborg@kernel.org \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=fujita.tomonori@gmail.com \
    --cc=gary@garyguo.net \
    --cc=jiangshanlai@gmail.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=ojeda@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=work@onurozkan.dev \
    /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.