linux-kernel - Re: [PATCH v2 2/5] rust: Add bindings for reading device properties

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] [day] [month] [year] [list]
Message-Id: <D9ETYGX6RS7R.1LLJX3WDZI4QY@buenzli.dev>
Date: Thu, 24 Apr 2025 13:25:28 +0200
From: "Remo Senekowitsch" <remo@...nzli.dev>
To: "Dirk Behme" <dirk.behme@...bosch.com>, "Rob Herring" <robh@...nel.org>,
 "Saravana Kannan" <saravanak@...gle.com>, "Miguel Ojeda"
 <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>, "Boqun Feng"
 <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Benno Lossin"
 <benno.lossin@...ton.me>, "Andreas Hindborg" <a.hindborg@...nel.org>,
 "Alice Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>,
 "Danilo Krummrich" <dakr@...nel.org>, "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>
Cc: <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
 <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH v2 2/5] rust: Add bindings for reading device properties

Hi Dirk,

On Wed Apr 23, 2025 at 2:29 PM CEST, Dirk Behme wrote:
> Hi Remo,
>
> On 14/04/2025 17:26, Remo Senekowitsch wrote:
>> The device property API is a firmware agnostic API for reading
>> properties from firmware (DT/ACPI) devices nodes and swnodes.
>> 
>> While the C API takes a pointer to a caller allocated variable/buffer,
>> the rust API is designed to return a value and can be used in struct
>> initialization. Rust generics are also utilized to support different
>> types of properties where appropriate.
>> 
>> The PropertyGuard is a way to force users to specify whether a property
>> is supposed to be required or not. This allows us to move error
>> logging of missing required properties into core, preventing a lot of
>> boilerplate in drivers.
>> 
>> Co-developed-by: Rob Herring (Arm) <robh@...nel.org>
>> Signed-off-by: Rob Herring (Arm) <robh@...nel.org>
>> Signed-off-by: Remo Senekowitsch <remo@...nzli.dev>
>> ---
>>  rust/kernel/property.rs | 385 +++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 383 insertions(+), 2 deletions(-)
>> 
>> diff --git a/rust/kernel/property.rs b/rust/kernel/property.rs
>> index f6e6c980d..0d4ea3168 100644
>> --- a/rust/kernel/property.rs
>> +++ b/rust/kernel/property.rs
> ...
>> +    /// helper used to display name or path of a fwnode
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// Callers must provide a valid format string for a fwnode.
>> +    unsafe fn fmt(&self, f: &mut core::fmt::Formatter<'_>, fmt_str: &CStr) -> core::fmt::Result {
>> +        let mut buf = [0; 256];
>> +        // SAFETY: `buf` is valid and `buf.len()` is its length. `self.as_raw()` is
>> +        // valid because `self` is valid.
>> +        let written = unsafe {
>> +            bindings::scnprintf(buf.as_mut_ptr(), buf.len(), fmt_str.as_ptr(), self.as_raw())
>> +        };
>> +        // SAFETY: `written` is smaller or equal to `buf.len()`.
>> +        let b: &[u8] = unsafe { core::slice::from_raw_parts(buf.as_ptr(), written as usize) };
>> +        write!(f, "{}", BStr::from_bytes(b))
>> +    }
>> +
>> +    /// Returns an object that implements [`Display`](core::fmt::Display) for
>> +    /// printing the name of a node.
>> +    pub fn display_name(&self) -> impl core::fmt::Display + use<'_> {
>
>
> I don't know about the details but with rustc 1.81 [1] I'm getting [2].
> Just doing what is proposed seems to "fix" it:
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 3b1af034e902e..eadf7501d499b 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -26,6 +26,7 @@
>  #![feature(const_mut_refs)]
>  #![feature(const_ptr_write)]
>  #![feature(const_refs_to_cell)]
> +#![feature(precise_capturing)]
>
>  // Ensure conditional compilation based on the kernel configuration works;
>  // otherwise we may silently break things like initcall handling.
>
> Just want to mention it because I think the minimal rustc version we
> have to support is 1.78.

Thanks for catching this! I'll make sure to compile with 1.78 from now
on. Luckily we don't need to activate an unstable feature, there is an
"older" capturing syntax that works here. (`'_` instead of `use<'_>`)

> Best regards
>
> Dirk
>
> P.S.: Many thanks for working on this! :)
>
> [1]
>
> $ rustc --version
> rustc 1.81.0 (eeb90cda1 2024-09-04)
>
> [2]
>
> error[E0658]: precise captures on `impl Trait` are experimental
>    --> rust/kernel/property.rs:256:61
>     |
> 256 |     pub fn display_name(&self) -> impl core::fmt::Display + use<'_> {
>     |                                                             ^^^
>     |
>     = note: see issue #123432
> <https://github.com/rust-lang/rust/issues/123432> for more information
>     = help: add `#![feature(precise_capturing)]` to the crate attributes
> to enable
>     = note: this compiler was built on 2024-09-04; consider upgrading it
> if it is out of date
>
> error[E0658]: precise captures on `impl Trait` are experimental
>    --> rust/kernel/property.rs:271:61
>     |
> 271 |     pub fn display_path(&self) -> impl core::fmt::Display + use<'_> {
>     |                                                             ^^^
>     |
>     = note: see issue #123432
> <https://github.com/rust-lang/rust/issues/123432> for more information
>     = help: add `#![feature(precise_capturing)]` to the crate attributes
> to enable
>     = note: this compiler was built on 2024-09-04; consider upgrading it
> if it is out of date
>
> error: aborting due to 2 previous errors

--
Best regards,
Remo