Message-ID: <CAGSQo02RH+-vK28O9HYkJHK5ymv0Pw=b0MhkJg5hkLdbLGJmmA@mail.gmail.com>
Date: Wed, 17 Dec 2025 09:57:19 -0800
From: Matthew Maurer <mmaurer@...gle.com>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, 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>, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] rust: Add support for deriving `AsBytes` and `FromBytes`

On Wed, Dec 17, 2025 at 9:36 AM Daniel Almeida
<daniel.almeida@...labora.com> wrote:
>
> Matthew,
>
> > On 15 Dec 2025, at 21:44, Matthew Maurer <mmaurer@...gle.com> wrote:
> >
> > This provides a derive macro for `AsBytes` and `FromBytes` for structs
> > only. For both, it checks the respective trait on every underlying
> > field. For `AsBytes`, it emits a const-time padding check that will fail
> > the compilation if derived on a type with padding.
> >
> > Signed-off-by: Matthew Maurer <mmaurer@...gle.com>
> > ---
> > rust/macros/lib.rs       | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
> > rust/macros/transmute.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 121 insertions(+)
> >
> > diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> > index b38002151871a33f6b4efea70be2deb6ddad38e2..d66397942529f67697f74a908e257cacc4201d84 100644
> > --- a/rust/macros/lib.rs
> > +++ b/rust/macros/lib.rs
> > @@ -20,9 +20,14 @@
> > mod kunit;
> > mod module;
> > mod paste;
> > +mod transmute;
> > mod vtable;
> >
> > use proc_macro::TokenStream;
> > +use syn::{
> > +    parse_macro_input,
> > +    DeriveInput, //
> > +};
> >
> > /// Declares a kernel module.
> > ///
> > @@ -475,3 +480,61 @@ pub fn paste(input: TokenStream) -> TokenStream {
> > pub fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
> >     kunit::kunit_tests(attr, ts)
> > }
> > +
> > +/// Implements `FromBytes` for a struct.
> > +///
> > +/// It will fail compilation if the struct you are deriving on cannot be determined to implement
> > +/// `FromBytes` safely. It may still fail for some types which would be safe to implement
> > +/// `FromBytes` for, in which case you will need to write the implementation and justification
> > +/// yourself.
> > +///
> > +/// Main reasons your type may be rejected:
> > +/// * Not a `struct`
> > +/// * One of the fields is not `FromBytes`
> > +///
> > +/// # Examples
> > +///
> > +/// ```
> > +/// #[derive(FromBytes)]
> > +/// #[repr(C)]
> > +/// struct Foo {
> > +///   x: u32,
> > +///   y: u16,
> > +///   z: u16,
> > +/// }
> > +/// ```
> > +#[proc_macro_derive(FromBytes)]
> > +pub fn derive_from_bytes(tokens: TokenStream) -> TokenStream {
> > +    let input = parse_macro_input!(tokens as DeriveInput);
> > +    transmute::from_bytes(input).into()
> > +}
> > +
> > +/// Implements `AsBytes` for a struct.
> > +///
> > +/// It will fail compilation if the struct you are deriving on cannot be determined to implement
> > +/// `AsBytes` safely. It may still fail for some structures which would be safe to implement
> > +/// `AsBytes`, in which case you will need to write the implementation and justification
> > +/// yourself.
> > +///
> > +/// Main reasons your type may be rejected:
> > +/// * Not a `struct`
> > +/// * One of the fields is not `AsBytes`
> > +/// * Your struct has generic parameters
> > +/// * There is padding somewhere in your struct
>
> Why is padding relevant here but not in FromBytes?

Padding bytes can be initialized to any value, but it is UB to observe
their contents because they may be implicitly uninitialized[1]. This
also matches the approach of `zerocopy`[2], which is the standard for
these sorts of transmutations outside the kernel - any byte can go
into the padding, but you cannot *read* any byte out of the padding.

[1]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html#r-undefined.validity.undef
[2]: https://docs.rs/zerocopy/latest/zerocopy/trait.FromBytes.html#warning-padding-bytes

>
> > +///
> > +/// # Examples
> > +///
> > +/// ```
> > +/// #[derive(AsBytes)]
> > +/// #[repr(C)]
> > +/// struct Foo {
> > +///   x: u32,
> > +///   y: u16,
> > +///   z: u16,
> > +/// }
> > +/// ```
> > +#[proc_macro_derive(AsBytes)]
> > +pub fn derive_as_bytes(tokens: TokenStream) -> TokenStream {
> > +    let input = parse_macro_input!(tokens as DeriveInput);
> > +    transmute::as_bytes(input).into()
> > +}
> > diff --git a/rust/macros/transmute.rs b/rust/macros/transmute.rs
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..43cf36a1334f1fed23c0e777026392f987f78d8d
> > --- /dev/null
> > +++ b/rust/macros/transmute.rs
> > @@ -0,0 +1,58 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +use proc_macro2::TokenStream;
> > +use syn::{parse_quote, DeriveInput, Fields, Ident, ItemConst, Path, WhereClause};
> > +
> > +fn all_fields_impl(fields: &Fields, trait_: &Path) -> WhereClause {
> > +    let tys = fields.iter().map(|field| &field.ty);
> > +    parse_quote! {
> > +        where #(for<'a> #tys: #trait_),*
>
> Why do we need this hrtb here?

It's a workaround to avoid needing `#![feature(trivial_bounds)]`.
Without it, generated code for, say, `where *const T: FromBytes` will
immediately cause a compilation error. With it, it will simply cause
the trait's requirements to be unfulfilled.

>
> > +    }
> > +}
> > +
> > +fn struct_padding_check(fields: &Fields, name: &Ident) -> ItemConst {
> > +    let tys = fields.iter().map(|field| &field.ty);
> > +    parse_quote! {
> > +        const _: () = {
> > +            assert!(#(core::mem::size_of::<#tys>())+* == core::mem::size_of::<#name>());
> > +        };
> > +    }
> > +}
> > +
> > +pub(crate) fn as_bytes(input: DeriveInput) -> TokenStream {
> > +    if !input.generics.params.is_empty() {
> > +        return quote::quote! { compile_error!("#[derive(AsBytes)] does not support generics") };
> > +    }
> > +    let syn::Data::Struct(ref ds) = &input.data else {
> > +        return quote::quote! { compile_error!("#[derive(AsBytes)] only supports structs") };
> > +    };
> > +    let name = input.ident;
> > +    let trait_ = parse_quote! { ::kernel::transmute::AsBytes };
> > +    let where_clause = all_fields_impl(&ds.fields, &trait_);
> > +    let padding_check = struct_padding_check(&ds.fields, &name);
> > +    quote::quote! {
> > +        #padding_check
> > +        // SAFETY: #name has no padding and all of its fields implement `AsBytes`
> > +        unsafe impl #trait_ for #name #where_clause {}
> > +    }
>
> In general I’d add blanks.

I'm not sure what you're suggesting here.

>
> > +}
> > +
> > +pub(crate) fn from_bytes(input: DeriveInput) -> TokenStream {
> > +    let syn::Data::Struct(ref ds) = &input.data else {
> > +        return quote::quote! { compile_error!("#[derive(FromBytes)] only supports structs") };
> > +    };
> > +    let (impl_generics, ty_generics, base_where_clause) = input.generics.split_for_impl();
> > +    let name = input.ident;
> > +    let trait_ = parse_quote! { ::kernel::transmute::FromBytes };
> > +    let mut where_clause = all_fields_impl(&ds.fields, &trait_);
> > +    if let Some(base_clause) = base_where_clause {
> > +        where_clause
> > +            .predicates
> > +            .extend(base_clause.predicates.clone())
> > +    };
> > +    quote::quote! {
> > +        // SAFETY: All fields of #name implement `FromBytes` and it is a struct, so there is no
> > +        // implicit discriminator.
> > +        unsafe impl #impl_generics #trait_ for #name #ty_generics #where_clause {}
> > +    }
> > +}
> >
> > --
> > 2.52.0.305.g3fc767764a-goog
> >
> >
>
> Overall looks good. Please chime in on the two questions above.
>
> — Daniel
>

Powered by blists - more mailing lists