linux-kernel - Re: [PATCH] rust/revocable: add try

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: <D8F8E4PBHK7O.399Y83M1L3XK3@nvidia.com>
Date: Fri, 14 Mar 2025 00:08:13 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Benno Lossin" <benno.lossin@...ton.me>, "Miguel Ojeda"
 <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>, "Boqun Feng"
 <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>, "Danilo Krummrich"
 <dakr@...nel.org>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Andreas Hindborg" <a.hindborg@...nel.org>,
 "Alice Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>
Cc: <rust-for-linux@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] rust/revocable: add try_with() convenience method

On Thu Mar 13, 2025 at 11:19 PM JST, Benno Lossin wrote:
> On Thu Mar 13, 2025 at 1:40 PM CET, Alexandre Courbot wrote:
>> diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
>> index 1e5a9d25c21b279b01f90b02997492aa4880d84f..0157b20373b5b2892cb618b46958bfe095e428b6 100644
>> --- a/rust/kernel/revocable.rs
>> +++ b/rust/kernel/revocable.rs
>> @@ -105,6 +105,28 @@ pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
>>          }
>>      }
>>  
>> +    /// Tries to access the wrapped object and run the closure `f` on it with the guard held.
>> +    ///
>> +    /// This is a convenience method to run short non-sleepable code blocks while ensuring the
>> +    /// guard is dropped afterwards. [`Self::try_access`] carries the risk that the caller
>> +    /// will forget to explicitly drop that returned guard before calling sleepable code ; this
>
> Space after `;`?
>
>> +    /// method adds an extra safety to make sure it doesn't happen.
>
> To be clear, you still can call a sleeping function form within the
> closure and have the same issue, but I agree that that should not happen
> accidentally (or at least not as often).

Yes, this is by no means a complete solution to the problem, just a way
to better cope with it.

>
>> +    ///
>> +    /// Returns `Err(ENXIO)` if the wrapped object has been revoked, or the result of `f` after it
>> +    /// has been run.
>> +    pub fn try_with<R, F: Fn(&T) -> Result<R>>(&self, f: F) -> Result<R> {
>
> This (and below) can be a `FnOnce(&T) -> Result<R>`.

Indeed, thanks!

>
> Would it make sense to not use `Result` here and continue with `Option`?

We would have to return an Option<Result<R>> in this case. The current
code folds the closure's Result into the one of the guard's acquisition
for convenience.

Actually, I don't think I have ever used try_access() a single time
without converting its returned Option into a Result. Wouldn't it make
sense to do the opposite, i.e. make try_access() return Err(ENXIO) when
the guard cannot be acquired and document this behavior?