linux-kernel - Re: [PATCH v2 1/8] rust: transmute: add `from_bytes

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DCDVBY8MOLWV.1N5ZPISMJL82O@nvidia.com>
Date: Thu, 28 Aug 2025 16:05:21 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "John Hubbard" <jhubbard@...dia.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" <lossin@...nel.org>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
 "Trevor Gross" <tmgross@...ch.edu>, "Danilo Krummrich" <dakr@...nel.org>,
 "David Airlie" <airlied@...il.com>, "Simona Vetter" <simona@...ll.ch>,
 "Maarten Lankhorst" <maarten.lankhorst@...ux.intel.com>, "Maxime Ripard"
 <mripard@...nel.org>, "Thomas Zimmermann" <tzimmermann@...e.de>
Cc: "Alistair Popple" <apopple@...dia.com>, "Joel Fernandes"
 <joelagnelf@...dia.com>, "Timur Tabi" <ttabi@...dia.com>,
 <rust-for-linux@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <nouveau@...ts.freedesktop.org>, <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v2 1/8] rust: transmute: add `from_bytes_copy` method to
 `FromBytes` trait

On Wed Aug 27, 2025 at 9:51 AM JST, John Hubbard wrote:
> On 8/25/25 9:07 PM, Alexandre Courbot wrote:
>> `FromBytes::from_bytes` comes with a few practical limitations:
>> 
>> - It requires the bytes slice to have the same alignment as the returned
>>   type, which might not be guaranteed in the case of a byte stream,
>> - It returns a reference, requiring the returned type to implement
>>   `Clone` if one wants to keep the value for longer than the lifetime of
>>   the slice.
>> 
>> To overcome these when needed, add a `from_bytes_copy` with a default
>> implementation in the trait. `from_bytes_copy` returns an owned value
>> that is populated using an unaligned read, removing the lifetime
>> constraint and making it usable even on non-aligned byte slices.
>> 
>> Reviewed-by: Alice Ryhl <aliceryhl@...gle.com>
>> Signed-off-by: Alexandre Courbot <acourbot@...dia.com>
>> ---
>>  rust/kernel/transmute.rs | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>> 
>> diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
>> index 494bb3b1d059337520efef694fc8952972d44fbf..721dd8254dcedd71ed7c1fc0ee9292950c16c89e 100644
>> --- a/rust/kernel/transmute.rs
>> +++ b/rust/kernel/transmute.rs
>> @@ -78,6 +78,23 @@ fn from_bytes_mut(bytes: &mut [u8]) -> Option<&mut Self>
>>              None
>>          }
>>      }
>> +
>> +    /// Creates an owned instance of `Self` by copying `bytes`.
>> +    ///
>> +    /// As the data is copied into a properly-aligned location, this method can be used even if
>> +    /// [`FromBytes::from_bytes`] would return `None` due to incompatible alignment.
>
> Some very minor suggestions:
>
> This wording less precise than it could be: "as the data is copied" can mean
> either "while it is being copied", or "because it is copied". Also, there
> should not be a hyphen in "properly aligned".
>
> I'd suggest something like this instead:
>  
>     /// Unlike [`FromBytes::from_bytes`], which requires aligned input, this
>     /// method can be used on non-aligned data.

That's much simpler and better. I'll just add "... at the cost of a
copy." to not lose this information.

>
>> +    fn from_bytes_copy(bytes: &[u8]) -> Option<Self>
>> +    where
>> +        Self: Sized,
>> +    {
>> +        if bytes.len() == size_of::<Self>() {
>> +            // SAFETY: `bytes` has the same size as `Self`, and per the invariants of `FromBytes`,
>> +            // any byte sequence is a valid value for `Self`.
>
> More wording suggestions. How about this:
>
>             // SAFETY: we just verified that `bytes` has the same size as `Self`, and per the
>             // invariants of `FromBytes`, any byte sequence of the correct length is a valid value
>             // for `Self`.

Taken as-is, thanks!