Skip to content

correct safety comments and documentation in revocable.rs #1160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
y86-dev opened this issue Apr 27, 2025 · 2 comments
Open

correct safety comments and documentation in revocable.rs #1160

y86-dev opened this issue Apr 27, 2025 · 2 comments
Labels
good first issue Good for newcomers • lib Related to the `rust/` library. medium Expected to be an issue of medium difficulty to resolve.

Comments

@y86-dev
Copy link
Member

y86-dev commented Apr 27, 2025

The Revocable type in rust/kernel/revocable.rs is missing an # Invariants section documenting that the value inside of data is valid if and only if is_available is true. This should be properly documented and then used in the appropriate places. Please also document the required usage of RCU.


This requires submitting a proper patch to the LKML and the Rust for Linux mailing list. Please recall to test your changes (including generating the documentation if changed, running the Rust doctests if changed, etc.), to use a proper title for the commit, to sign your commit under the Developer's Certificate of Origin and to add a Suggested-by: tag, and a Link: tag to this issue. Please see https://docs.kernel.org/process/submitting-patches.html and https://rust-for-linux.com/contributing for details.

@y86-dev y86-dev added medium Expected to be an issue of medium difficulty to resolve. • lib Related to the `rust/` library. labels Apr 27, 2025
@ojeda ojeda changed the title correct safety comments and documentation in reovocable.rs correct safety comments and documentation in revocable.rs Apr 27, 2025
@ojeda ojeda added the good first issue Good for newcomers label Apr 27, 2025
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue May 1, 2025
Clarify the safety invariants for `Revocable<T>`, documenting that `data`
is only valid if `is_available` is `true`, and that access requires
holding the RCU read-side lock.

This helps developers understand when it is safe to access the wrapped
data and the role of RCU in ensuring memory safety.

This is part of issue torvalds#1160 ("correct safety comments and documentation
in revocable.rs"), reported by Miguel Ojeda and Benno Lossin.

Reported-by: Miguel Ojeda <[email protected]>
Closes: Rust-for-Linux#1160
Reported-by: Benno Lossin <[email protected]>
Closes: Rust-for-Linux#1160
Tested-by: Marcelo Moreira <[email protected]>
Signed-off-by: Marcelo Moreira <[email protected]>
@meritissimo1
Copy link

Hi guys ✌️

I've submitted a patch that addresses this issue by adding a snippet to the documentation about security invariants for Revocable. You can find it here:

🔗 https://lore.kernel.org/rust-for-linux/[email protected]/T/#u

Happy to hear any feedback! 🙂

In Brazil we say TAMO JUNTO! 🙂

@meritissimo1
Copy link

The v2:

[PATCH v2]

Tkss! =)

intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue May 26, 2025
Introducing a comprehensive `# Invariants` section for the `Revocable<T>`
type. This new documentation details the validity conditions of the `data`
field concerning `is_available` and RCU read-side locks.

The safety comments in `Revocable::try_access`,
`Revocable::try_access_with_guard`, and the `PinnedDrop` implementation
for `Revocable<T>` have been updated to explicitly reference these newly
defined invariants, ensuring that the justification for unsafe operations
is clear and directly tied to the type's guaranteed properties.

Reported-by: Benno Lossin <[email protected]>
Closes: Rust-for-Linux#1160
Suggested-by: Benno Lossin <[email protected]>
Suggested-by: Danilo Krummrich <[email protected]>
Signed-off-by: Marcelo Moreira <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers • lib Related to the `rust/` library. medium Expected to be an issue of medium difficulty to resolve.
Development

No branches or pull requests

3 participants