mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Daniel Almeida <daniel.almeida@collabora.com>
To: Matthew Maurer <mmaurer@google.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/3] rust: Add support for deriving `AsBytes` and `FromBytes`
Date: Wed, 17 Dec 2025 14:35:59 -0300	[thread overview]
Message-ID: <E3281FEE-EE5E-4594-9B0E-E9350BA2136D@collabora.com> (raw)
In-Reply-To: <20251216-transmute-v2-2-b23e5277ad02@google.com>

Matthew,

> On 15 Dec 2025, at 21:44, Matthew Maurer <mmaurer@google.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@google.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?

> +///
> +/// # 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?

> +    }
> +}
> +
> +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.

> +}
> +
> +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


  parent reply	other threads:[~2025-12-17 17:36 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-16  0:44 [PATCH v2 0/3] Support more safe `AsBytes`/`FromBytes` usage Matthew Maurer
2025-12-16  0:44 ` [PATCH v2 1/3] rust: transmute: Support transmuting slices of AsBytes/FromBytes types Matthew Maurer
2025-12-17 16:51   ` Daniel Almeida
2025-12-26 20:27     ` Matthew Maurer
2025-12-16  0:44 ` [PATCH v2 2/3] rust: Add support for deriving `AsBytes` and `FromBytes` Matthew Maurer
2025-12-17  3:12   ` Alexandre Courbot
2025-12-17 18:01     ` Matthew Maurer
2025-12-17 19:14       ` Daniel Almeida
2025-12-18  7:23       ` Alexandre Courbot
2025-12-18  8:26         ` Alice Ryhl
2025-12-17 17:35   ` Daniel Almeida [this message]
2025-12-17 17:57     ` Matthew Maurer
2025-12-17 19:11       ` Daniel Almeida
2025-12-16  0:44 ` [PATCH v2 3/3] rust: Support deriving `AsBytes`/`FromBytes` on bindgen types Matthew Maurer
2025-12-17  3:15   ` Alexandre Courbot
2025-12-17 18:26     ` Matthew Maurer
2025-12-17 19:26   ` Daniel Almeida
2025-12-17 19:33     ` Matthew Maurer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E3281FEE-EE5E-4594-9B0E-E9350BA2136D@collabora.com \
    --to=daniel.almeida@collabora.com \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=mmaurer@google.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®