linux-kernel - Re: [PATCH v5 0/2] rust: revocable: documentation and refactorings

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: <CAPZ3m_iqCRYh+XhMip3h=ZWKpw4VPfnRQ6ofmoAnrzKDbOO-PA@mail.gmail.com>
Date: Sat, 5 Jul 2025 02:09:25 -0300
From: Marcelo Moreira <marcelomoreira1905@...il.com>
To: Benno Lossin <lossin@...nel.org>
Cc: rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org, 
	dakr@...nel.org, ojeda@...nel.org, skhan@...uxfoundation.org, 
	linux-kernel-mentees@...ts.linuxfoundation.org, ~lkcamp/patches@...ts.sr.ht
Subject: Re: [PATCH v5 0/2] rust: revocable: documentation and refactorings

Em qui., 3 de jul. de 2025 às 05:24, Benno Lossin <lossin@...nel.org> escreveu:
>
> On Thu Jun 26, 2025 at 6:59 PM CEST, Marcelo Moreira wrote:
> > This patch series brings documentation and refactorings to the `Revocable` type.
> >
> > Changes include:
> > - Clarifying the write invariant and updating associated safety comments for `Revocable<T>`.
> > - Splitting the internal `revoke_internal` function into two distinct, explicit functions: `revoke()` (safe, synchronizing with RCU) and `revoke_nosync()` (unsafe, without RCU synchronization), now returning `bool` to indicate revocation status.
>
> Could you wrap your text to a more readable column? Thanks!

Sure! Thanks!

>
> >
> > Marcelo Moreira (2):
> >   rust: revocable: Refactor revocation mechanism to remove generic
> >     revoke_internal
> >   rust: revocable: Clarify write invariant and update safety comments
> >
> > Changelog
> > ---------
> >
> > Changes since v4:
> > - Rebased the series onto the latest `rfl/rust-next` to integrate recent changes, specifically the `bool` return for `revoke()` and `revoke_nosync()`.
> > - Dropped the "rust: revocable: simplify RevocableGuard for internal safety" patch, as the approach of using a direct reference (`&'a T`) for `RevocableGuard` was found to be unsound due to Rust's aliasing rules and LLVM's `dereferencable` attribute guarantees, which require references to remain valid for the entire function call duration, even if the internal RCU guard is dropped earlier.
> > - Refined the `PinnedDrop::drop` `SAFETY` comment based on Benno Lossin's and Miguel Ojeda's feedback, adopting a more concise and standard Kernel-style bullet point format.
> > - Corrected a duplicated line in the commit message of the second patch.
>
> Now since we had to drop the `RevocableGuard` change, its safety
> invariant & comment in `deref` is insufficient. It shouldn't have the
> invariant that the rcu lock is held (since it owns an `rcu::Guard`, that
> already is guaranteed), but instead it should require that the
> `data_ref` pointer is valid. That invariant is then used by the safety
> comment in `deref` to justify dereferencing the pointer.
>
> Also, I think it's better to reorder the patches again (since the
> current first one relies on changes from the second one), the first one
> should be the change to the invariants section of `Revocable` (so
> currently the second patch). Then the second and third patches can be
> the removal of `revoke_internal` and the `RevocableGuard` safety
> documentation fix.

All right Benno, I'll prepare the comment for `RevocableGuard` and send v6.

The order now is:
1- Documentation for invariant and updates associated `SAFETY` comments
2- Remove `revoke_internal` (Refactoring)
3- `RevocableGuard` safety documentation fix.

Thanks! :)

--
Cheers,
Marcelo Moreira