public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] rust: pin-init: internal: init: remove `#[disable_initialized_field_access]`
@ 2026-02-28 11:37 Benno Lossin
  2026-02-28 11:37 ` [PATCH 2/2] rust: pin-init: internal: init: document load-bearing fact of field accessors Benno Lossin
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Benno Lossin @ 2026-02-28 11:37 UTC (permalink / raw)
  To: Benno Lossin, Gary Guo, Miguel Ojeda, Boqun Feng,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich
  Cc: Janne Grunau, asahi, rust-for-linux, linux-kernel

Gary noticed [1] that the initializer macros as well as the `[Pin]Init`
traits cannot support packed struct, since they use operations that
require aligned pointers. This means that any code using packed structs
and pin-init is unsound.

Thus remove the `#[disable_initialized_field_access]` attribute from
`init!`, which is the only safe way to create an initializer of a packed
struct.

In the future, we can add support for packed structs by changing the
trait infrastructure to include `UnalignedInit` or hopefully another
mechanism.

Reported-by: Gary Guo <gary@garyguo.net>
Link: https://rust-for-linux.zulipchat.com/#narrow/channel/561532-pin-init/topic/initialized.20field.20accessor.20detection/with/576210658 [1]
Fixes: ceca298c53f9 ("rust: pin-init: internal: init: add escape hatch for referencing initialized fields")
Signed-off-by: Benno Lossin <lossin@kernel.org>
---
This commit does not need backporting, as ceca298c53f9 is not yet in any
stable tree.

However, the unsoundness still affects several stable trees, because it
was unknowingly fixed in commit 42415d163e5d ("rust: pin-init: add
references to previously initialized fields"). Before then, packed
structs compiled without any issues with pin-init and thus all prior
kernel versions with pin-init that do not contain that commit are
affected.

We introduced pin-init in 90e53c5e70a6 ("rust: add pin-init API core"),
which was included in 6.4. The affected stable trees that are still
maintained are: 6.17, 6.16, 6.12, and 6.6. Note that 6.18 and 6.19
already contain 42415d163e5d, so they are unaffected.

I will prepare a separate patch series to backport 42415d163e5d to each
of the affected trees, including the second patch of this series that
documents the fact that field accessors are load-bearing for soundness.

@asahi folks, let me know if I should prioritize a solution for packed
structs. Otherwise I'd like not support it at the moment, as that
requires some deeper changes to the internals of pin-init. I'm tracking
the status of packed structs in:

    https://github.com/Rust-for-Linux/pin-init/issues/112
---
 rust/pin-init/internal/src/init.rs | 39 ++++++------------------------
 1 file changed, 8 insertions(+), 31 deletions(-)

diff --git a/rust/pin-init/internal/src/init.rs b/rust/pin-init/internal/src/init.rs
index 42936f915a07..da53adc44ecf 100644
--- a/rust/pin-init/internal/src/init.rs
+++ b/rust/pin-init/internal/src/init.rs
@@ -62,7 +62,6 @@ fn ident(&self) -> Option<&Ident> {
 
 enum InitializerAttribute {
     DefaultError(DefaultErrorAttribute),
-    DisableInitializedFieldAccess,
 }
 
 struct DefaultErrorAttribute {
@@ -86,6 +85,7 @@ pub(crate) fn expand(
     let error = error.map_or_else(
         || {
             if let Some(default_error) = attrs.iter().fold(None, |acc, attr| {
+                #[expect(irrefutable_let_patterns)]
                 if let InitializerAttribute::DefaultError(DefaultErrorAttribute { ty }) = attr {
                     Some(ty.clone())
                 } else {
@@ -145,15 +145,7 @@ fn assert_zeroable<T: ?::core::marker::Sized>(_: *mut T)
     };
     // `mixed_site` ensures that the data is not accessible to the user-controlled code.
     let data = Ident::new("__data", Span::mixed_site());
-    let init_fields = init_fields(
-        &fields,
-        pinned,
-        !attrs
-            .iter()
-            .any(|attr| matches!(attr, InitializerAttribute::DisableInitializedFieldAccess)),
-        &data,
-        &slot,
-    );
+    let init_fields = init_fields(&fields, pinned, &data, &slot);
     let field_check = make_field_check(&fields, init_kind, &path);
     Ok(quote! {{
         // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
@@ -236,7 +228,6 @@ fn get_init_kind(rest: Option<(Token![..], Expr)>, dcx: &mut DiagCtxt) -> InitKi
 fn init_fields(
     fields: &Punctuated<InitializerField, Token![,]>,
     pinned: bool,
-    generate_initialized_accessors: bool,
     data: &Ident,
     slot: &Ident,
 ) -> TokenStream {
@@ -272,13 +263,6 @@ fn init_fields(
                         unsafe { &mut (*#slot).#ident }
                     }
                 };
-                let accessor = generate_initialized_accessors.then(|| {
-                    quote! {
-                        #(#cfgs)*
-                        #[allow(unused_variables)]
-                        let #ident = #accessor;
-                    }
-                });
                 quote! {
                     #(#attrs)*
                     {
@@ -286,7 +270,9 @@ fn init_fields(
                         // SAFETY: TODO
                         unsafe { #write(::core::ptr::addr_of_mut!((*#slot).#ident), #value_ident) };
                     }
-                    #accessor
+                    #(#cfgs)*
+                    #[allow(unused_variables)]
+                    let #ident = #accessor;
                 }
             }
             InitializerKind::Init { ident, value, .. } => {
@@ -326,20 +312,15 @@ fn init_fields(
                         },
                     )
                 };
-                let accessor = generate_initialized_accessors.then(|| {
-                    quote! {
-                        #(#cfgs)*
-                        #[allow(unused_variables)]
-                        let #ident = #accessor;
-                    }
-                });
                 quote! {
                     #(#attrs)*
                     {
                         let #init = #value;
                         #value_init
                     }
-                    #accessor
+                    #(#cfgs)*
+                    #[allow(unused_variables)]
+                    let #ident = #accessor;
                 }
             }
             InitializerKind::Code { block: value, .. } => quote! {
@@ -466,10 +447,6 @@ fn parse(input: syn::parse::ParseStream<'_>) -> syn::Result<Self> {
                 if a.path().is_ident("default_error") {
                     a.parse_args::<DefaultErrorAttribute>()
                         .map(InitializerAttribute::DefaultError)
-                } else if a.path().is_ident("disable_initialized_field_access") {
-                    a.meta
-                        .require_path_only()
-                        .map(|_| InitializerAttribute::DisableInitializedFieldAccess)
                 } else {
                     Err(syn::Error::new_spanned(a, "unknown initializer attribute"))
                 }

base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] rust: pin-init: internal: init: document load-bearing fact of field accessors
  2026-02-28 11:37 [PATCH 1/2] rust: pin-init: internal: init: remove `#[disable_initialized_field_access]` Benno Lossin
@ 2026-02-28 11:37 ` Benno Lossin
  2026-02-28 11:55   ` Gary Guo
  2026-02-28 14:11   ` Miguel Ojeda
  2026-02-28 12:03 ` [PATCH 1/2] rust: pin-init: internal: init: remove `#[disable_initialized_field_access]` Gary Guo
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 10+ messages in thread
From: Benno Lossin @ 2026-02-28 11:37 UTC (permalink / raw)
  To: Benno Lossin, Gary Guo, Miguel Ojeda, Boqun Feng,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Wedson Almeida Filho
  Cc: stable, rust-for-linux, linux-kernel

We cannot support packed structs without significant changes [1]. The
field accessors ensure that the compiler emits an error if one tries to
create an initializer for a packed struct.

Link: https://github.com/Rust-for-Linux/pin-init/issues/112 [1]
Fixes: 90e53c5e70a6 ("rust: add pin-init API core")
Cc: stable@vger.kernel.org # needed in 6.19, 6.18, 6.17, 6.16, 6.12, 6.6. see below the `---` for more info
Signed-off-by: Benno Lossin <lossin@kernel.org>
---
As already explained in the previous email, we discovered an unsoundness
in pin-init that exists since the beginning, but was unknowingly fixed
in commit 42415d163e5d ("rust: pin-init: add references to previously
initialized fields").

We introduced pin-init in 90e53c5e70a6 ("rust: add pin-init API core"),
which was included in 6.4. The affected stable trees that are still
maintained are: 6.17, 6.16, 6.12, and 6.6. Note that 6.18 and 6.19
already contain 42415d163e5d, so they are unaffected.

We still should backport this piece of documentation explaining the need
for the field accessors for soundness. For this reasons we also want to
backport it to 6.18 and 6.19.

Note that this patch depends on 42415d163e5d; so the only versions this
patch can go in directly are 6.18 and 6.19. I will send separate patch
series' for the older versions. The series' will include a backport of
42415d163e5d as well as this patch, since this patch depends on the
`syn` rewrite, which is not present in older versions.
---
 rust/pin-init/internal/src/init.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/rust/pin-init/internal/src/init.rs b/rust/pin-init/internal/src/init.rs
index da53adc44ecf..533029d53d30 100644
--- a/rust/pin-init/internal/src/init.rs
+++ b/rust/pin-init/internal/src/init.rs
@@ -251,6 +251,11 @@ fn init_fields(
                 });
                 // Again span for better diagnostics
                 let write = quote_spanned!(ident.span()=> ::core::ptr::write);
+                // NOTE: the field accessor ensures that the initialized struct is not
+                // `repr(packed)`. If it were, the compiler would emit E0793. We do not support
+                // packed structs, since `Init::__init` requires an aligned pointer; the same
+                // requirement that the call to `ptr::write` below has.
+                // For more info see <https://github.com/Rust-for-Linux/pin-init/issues/112>
                 let accessor = if pinned {
                     let project_ident = format_ident!("__project_{ident}");
                     quote! {
@@ -278,6 +283,11 @@ fn init_fields(
             InitializerKind::Init { ident, value, .. } => {
                 // Again span for better diagnostics
                 let init = format_ident!("init", span = value.span());
+                // NOTE: the field accessor ensures that the initialized struct is not
+                // `repr(packed)`. If it were, the compiler would emit E0793. We do not support
+                // packed structs, since `Init::__init` requires an aligned pointer; the same
+                // requirement that the call to `ptr::write` below has.
+                // For more info see <https://github.com/Rust-for-Linux/pin-init/issues/112>
                 let (value_init, accessor) = if pinned {
                     let project_ident = format_ident!("__project_{ident}");
                     (
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] rust: pin-init: internal: init: document load-bearing fact of field accessors
  2026-02-28 11:37 ` [PATCH 2/2] rust: pin-init: internal: init: document load-bearing fact of field accessors Benno Lossin
@ 2026-02-28 11:55   ` Gary Guo
  2026-02-28 14:56     ` Benno Lossin
  2026-02-28 14:11   ` Miguel Ojeda
  1 sibling, 1 reply; 10+ messages in thread
From: Gary Guo @ 2026-02-28 11:55 UTC (permalink / raw)
  To: Benno Lossin, Gary Guo, Miguel Ojeda, Boqun Feng,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Wedson Almeida Filho
  Cc: stable, rust-for-linux, linux-kernel

On Sat Feb 28, 2026 at 11:37 AM GMT, Benno Lossin wrote:
> We cannot support packed structs without significant changes [1]. The
> field accessors ensure that the compiler emits an error if one tries to
> create an initializer for a packed struct.
>
> Link: https://github.com/Rust-for-Linux/pin-init/issues/112 [1]
> Fixes: 90e53c5e70a6 ("rust: add pin-init API core")
> Cc: stable@vger.kernel.org # needed in 6.19, 6.18, 6.17, 6.16, 6.12, 6.6. see below the `---` for more info
> Signed-off-by: Benno Lossin <lossin@kernel.org>
> ---
> As already explained in the previous email, we discovered an unsoundness
> in pin-init that exists since the beginning, but was unknowingly fixed
> in commit 42415d163e5d ("rust: pin-init: add references to previously
> initialized fields").
>
> We introduced pin-init in 90e53c5e70a6 ("rust: add pin-init API core"),
> which was included in 6.4. The affected stable trees that are still
> maintained are: 6.17, 6.16, 6.12, and 6.6. Note that 6.18 and 6.19
> already contain 42415d163e5d, so they are unaffected.
>
> We still should backport this piece of documentation explaining the need
> for the field accessors for soundness. For this reasons we also want to
> backport it to 6.18 and 6.19.
>
> Note that this patch depends on 42415d163e5d; so the only versions this
> patch can go in directly are 6.18 and 6.19. I will send separate patch
> series' for the older versions. The series' will include a backport of
> 42415d163e5d as well as this patch, since this patch depends on the
> `syn` rewrite, which is not present in older versions.
> ---
>  rust/pin-init/internal/src/init.rs | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/rust/pin-init/internal/src/init.rs b/rust/pin-init/internal/src/init.rs
> index da53adc44ecf..533029d53d30 100644
> --- a/rust/pin-init/internal/src/init.rs
> +++ b/rust/pin-init/internal/src/init.rs
> @@ -251,6 +251,11 @@ fn init_fields(
>                  });
>                  // Again span for better diagnostics
>                  let write = quote_spanned!(ident.span()=> ::core::ptr::write);
> +                // NOTE: the field accessor ensures that the initialized struct is not
> +                // `repr(packed)`. If it were, the compiler would emit E0793. We do not support
> +                // packed structs, since `Init::__init` requires an aligned pointer; the same
> +                // requirement that the call to `ptr::write` below has.
> +                // For more info see <https://github.com/Rust-for-Linux/pin-init/issues/112>

The emphasis should be unaligned fields instead of `repr(packed)`. Of course,
unaligned fields can only occur with `repr(packed)`, but packed structs can
contain well-aligned fields, too (e.g. 1-byte aligned members, or
`repr(packed(2))` with 2-byte aligned members, etc...). Rust permits creation of
references to these fields.

Something like:

    NOTE: the field accessor ensures that the initialized field is properly
    aligned. Unaligned fields will cause the compiler to emit E0793. We do not
    support unaligned fields since `Init::__init` requires an aligned pointer;
    the `ptr::write` below has the same requirement.

Also, it is not immediately clear to me which one, buyt one of the two occurance
should be `PinInit::__pin_init`?

Best,
Gary

>                  let accessor = if pinned {
>                      let project_ident = format_ident!("__project_{ident}");
>                      quote! {
> @@ -278,6 +283,11 @@ fn init_fields(
>              InitializerKind::Init { ident, value, .. } => {
>                  // Again span for better diagnostics
>                  let init = format_ident!("init", span = value.span());
> +                // NOTE: the field accessor ensures that the initialized struct is not
> +                // `repr(packed)`. If it were, the compiler would emit E0793. We do not support
> +                // packed structs, since `Init::__init` requires an aligned pointer; the same
> +                // requirement that the call to `ptr::write` below has.
> +                // For more info see <https://github.com/Rust-for-Linux/pin-init/issues/112>
>                  let (value_init, accessor) = if pinned {
>                      let project_ident = format_ident!("__project_{ident}");
>                      (


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] rust: pin-init: internal: init: remove `#[disable_initialized_field_access]`
  2026-02-28 11:37 [PATCH 1/2] rust: pin-init: internal: init: remove `#[disable_initialized_field_access]` Benno Lossin
  2026-02-28 11:37 ` [PATCH 2/2] rust: pin-init: internal: init: document load-bearing fact of field accessors Benno Lossin
@ 2026-02-28 12:03 ` Gary Guo
  2026-02-28 14:09 ` Miguel Ojeda
  2026-03-01 17:12 ` Janne Grunau
  3 siblings, 0 replies; 10+ messages in thread
From: Gary Guo @ 2026-02-28 12:03 UTC (permalink / raw)
  To: Benno Lossin, Gary Guo, Miguel Ojeda, Boqun Feng,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich
  Cc: Janne Grunau, asahi, rust-for-linux, linux-kernel

On Sat Feb 28, 2026 at 11:37 AM GMT, Benno Lossin wrote:
> Gary noticed [1] that the initializer macros as well as the `[Pin]Init`
> traits cannot support packed struct, since they use operations that

This is better read as "unaligned field", rather than "packed struct". The
packed struct can be initialized as a whole by moving it into place.

After all, a value of a packed struct is itself a valid initializer and is
sound.

If you have

#[derive(Zeroable)]
struct Foo {
   x: u8,
   y: u16,
   z: u8,
}

it is also valid to have `pin_init!(Foo { x: 1, ..Zeroable::zeroed() })`

> require aligned pointers. This means that any code using packed structs
> and pin-init is unsound.
> 
> Thus remove the `#[disable_initialized_field_access]` attribute from
> `init!`, which is the only safe way to create an initializer of a packed
> struct.
> 
> In the future, we can add support for packed structs by changing the
> trait infrastructure to include `UnalignedInit` or hopefully another
> mechanism.

I guess the comment can be reworded a bit, to hint that this won't be added
unless there's a compelling need, e.g.

    If support for in-place initializing packed structs is required in the
    future, we can ...

Personally I think this won't be neeeded, as there are many ways around
supporting unaligned fields, e.g. by creating and moving the entire packed
structs. Another way is to use `Unaligned<T>` type for fields that needs
pin-init, similar to what zerocopy have.

Best,
Gary

> 
> Reported-by: Gary Guo <gary@garyguo.net>
> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/561532-pin-init/topic/initialized.20field.20accessor.20detection/with/576210658 [1]
> Fixes: ceca298c53f9 ("rust: pin-init: internal: init: add escape hatch for referencing initialized fields")
> Signed-off-by: Benno Lossin <lossin@kernel.org>
> ---
> This commit does not need backporting, as ceca298c53f9 is not yet in any
> stable tree.
> 
> However, the unsoundness still affects several stable trees, because it
> was unknowingly fixed in commit 42415d163e5d ("rust: pin-init: add
> references to previously initialized fields"). Before then, packed
> structs compiled without any issues with pin-init and thus all prior
> kernel versions with pin-init that do not contain that commit are
> affected.
> 
> We introduced pin-init in 90e53c5e70a6 ("rust: add pin-init API core"),
> which was included in 6.4. The affected stable trees that are still
> maintained are: 6.17, 6.16, 6.12, and 6.6. Note that 6.18 and 6.19
> already contain 42415d163e5d, so they are unaffected.
> 
> I will prepare a separate patch series to backport 42415d163e5d to each
> of the affected trees, including the second patch of this series that
> documents the fact that field accessors are load-bearing for soundness.
> 
> @asahi folks, let me know if I should prioritize a solution for packed
> structs. Otherwise I'd like not support it at the moment, as that
> requires some deeper changes to the internals of pin-init. I'm tracking
> the status of packed structs in:
> 
>     https://github.com/Rust-for-Linux/pin-init/issues/112
> ---
>  rust/pin-init/internal/src/init.rs | 39 ++++++------------------------
>  1 file changed, 8 insertions(+), 31 deletions(-)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] rust: pin-init: internal: init: remove `#[disable_initialized_field_access]`
  2026-02-28 11:37 [PATCH 1/2] rust: pin-init: internal: init: remove `#[disable_initialized_field_access]` Benno Lossin
  2026-02-28 11:37 ` [PATCH 2/2] rust: pin-init: internal: init: document load-bearing fact of field accessors Benno Lossin
  2026-02-28 12:03 ` [PATCH 1/2] rust: pin-init: internal: init: remove `#[disable_initialized_field_access]` Gary Guo
@ 2026-02-28 14:09 ` Miguel Ojeda
  2026-03-01 17:12 ` Janne Grunau
  3 siblings, 0 replies; 10+ messages in thread
From: Miguel Ojeda @ 2026-02-28 14:09 UTC (permalink / raw)
  To: Benno Lossin
  Cc: Gary Guo, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Janne Grunau, asahi, rust-for-linux, linux-kernel

On Sat, Feb 28, 2026 at 12:37 PM Benno Lossin <lossin@kernel.org> wrote:
>
> The affected stable trees that are still
> maintained are: 6.17, 6.16, 6.12, and 6.6.

6.17 and 6.16 are not maintained, so we can ignore those, i.e. only
6.12 and 6.6 will need it.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] rust: pin-init: internal: init: document load-bearing fact of field accessors
  2026-02-28 11:37 ` [PATCH 2/2] rust: pin-init: internal: init: document load-bearing fact of field accessors Benno Lossin
  2026-02-28 11:55   ` Gary Guo
@ 2026-02-28 14:11   ` Miguel Ojeda
  2026-02-28 14:49     ` Benno Lossin
  1 sibling, 1 reply; 10+ messages in thread
From: Miguel Ojeda @ 2026-02-28 14:11 UTC (permalink / raw)
  To: Benno Lossin
  Cc: Gary Guo, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Wedson Almeida Filho, stable, rust-for-linux, linux-kernel

On Sat, Feb 28, 2026 at 12:37 PM Benno Lossin <lossin@kernel.org> wrote:
>
> The affected stable trees that are still
> maintained are: 6.17, 6.16, 6.12, and 6.6.

Same here, i.e. 6.17 and 6.16 are not maintained anymore, so these can
be skipped.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] rust: pin-init: internal: init: document load-bearing fact of field accessors
  2026-02-28 14:11   ` Miguel Ojeda
@ 2026-02-28 14:49     ` Benno Lossin
  0 siblings, 0 replies; 10+ messages in thread
From: Benno Lossin @ 2026-02-28 14:49 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Gary Guo, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Wedson Almeida Filho, stable, rust-for-linux, linux-kernel

On Sat Feb 28, 2026 at 3:11 PM CET, Miguel Ojeda wrote:
> On Sat, Feb 28, 2026 at 12:37 PM Benno Lossin <lossin@kernel.org> wrote:
>>
>> The affected stable trees that are still
>> maintained are: 6.17, 6.16, 6.12, and 6.6.
>
> Same here, i.e. 6.17 and 6.16 are not maintained anymore, so these can
> be skipped.

Oh perfect, that means less work then. I wonder where I saw these, since
I checked the website, but now I of course don't see them there...

Cheers,
Benno

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] rust: pin-init: internal: init: document load-bearing fact of field accessors
  2026-02-28 11:55   ` Gary Guo
@ 2026-02-28 14:56     ` Benno Lossin
  0 siblings, 0 replies; 10+ messages in thread
From: Benno Lossin @ 2026-02-28 14:56 UTC (permalink / raw)
  To: Gary Guo, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Wedson Almeida Filho
  Cc: stable, rust-for-linux, linux-kernel

On Sat Feb 28, 2026 at 12:55 PM CET, Gary Guo wrote:
> On Sat Feb 28, 2026 at 11:37 AM GMT, Benno Lossin wrote:
>> diff --git a/rust/pin-init/internal/src/init.rs b/rust/pin-init/internal/src/init.rs
>> index da53adc44ecf..533029d53d30 100644
>> --- a/rust/pin-init/internal/src/init.rs
>> +++ b/rust/pin-init/internal/src/init.rs
>> @@ -251,6 +251,11 @@ fn init_fields(
>>                  });
>>                  // Again span for better diagnostics
>>                  let write = quote_spanned!(ident.span()=> ::core::ptr::write);
>> +                // NOTE: the field accessor ensures that the initialized struct is not
>> +                // `repr(packed)`. If it were, the compiler would emit E0793. We do not support
>> +                // packed structs, since `Init::__init` requires an aligned pointer; the same
>> +                // requirement that the call to `ptr::write` below has.
>> +                // For more info see <https://github.com/Rust-for-Linux/pin-init/issues/112>
>
> The emphasis should be unaligned fields instead of `repr(packed)`. Of course,
> unaligned fields can only occur with `repr(packed)`, but packed structs can
> contain well-aligned fields, too (e.g. 1-byte aligned members, or
> `repr(packed(2))` with 2-byte aligned members, etc...). Rust permits creation of
> references to these fields.

Yeah that's a more accurate account of things.

> Something like:
>
>     NOTE: the field accessor ensures that the initialized field is properly
>     aligned. Unaligned fields will cause the compiler to emit E0793. We do not
>     support unaligned fields since `Init::__init` requires an aligned pointer;
>     the `ptr::write` below has the same requirement.

That's a much better suggestion, I'll send an updated series later
today.

> Also, it is not immediately clear to me which one, buyt one of the two occurance
> should be `PinInit::__pin_init`?

No, `PinInit::__pin_init` is never called from the macro, since that
only makes sense for structurally pinned fields. That info isn't
available at the callsite of `init!`. We emit it in `#[pin_data]` which
exposes it to `init!` via the `PinData`. That ZST has a method with the
same name as the field and it takes the respective initializer (so `impl
Init` or `impl PinInit`) and just runs said initializer.

This happens in the second hunk in the case where `pinned == true`.

Cheers,
Benno

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] rust: pin-init: internal: init: remove `#[disable_initialized_field_access]`
  2026-02-28 11:37 [PATCH 1/2] rust: pin-init: internal: init: remove `#[disable_initialized_field_access]` Benno Lossin
                   ` (2 preceding siblings ...)
  2026-02-28 14:09 ` Miguel Ojeda
@ 2026-03-01 17:12 ` Janne Grunau
  2026-03-02 14:11   ` Benno Lossin
  3 siblings, 1 reply; 10+ messages in thread
From: Janne Grunau @ 2026-03-01 17:12 UTC (permalink / raw)
  To: Benno Lossin
  Cc: Gary Guo, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	asahi, rust-for-linux, linux-kernel

On Sat, Feb 28, 2026 at 12:37:04PM +0100, Benno Lossin wrote:
> Gary noticed [1] that the initializer macros as well as the `[Pin]Init`
> traits cannot support packed struct, since they use operations that
> require aligned pointers. This means that any code using packed structs
> and pin-init is unsound.
> 
> Thus remove the `#[disable_initialized_field_access]` attribute from
> `init!`, which is the only safe way to create an initializer of a packed
> struct.
> 
> In the future, we can add support for packed structs by changing the
> trait infrastructure to include `UnalignedInit` or hopefully another
> mechanism.
> 
> Reported-by: Gary Guo <gary@garyguo.net>
> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/561532-pin-init/topic/initialized.20field.20accessor.20detection/with/576210658 [1]
> Fixes: ceca298c53f9 ("rust: pin-init: internal: init: add escape hatch for referencing initialized fields")
> Signed-off-by: Benno Lossin <lossin@kernel.org>
> ---
> This commit does not need backporting, as ceca298c53f9 is not yet in any
> stable tree.
> 
> However, the unsoundness still affects several stable trees, because it
> was unknowingly fixed in commit 42415d163e5d ("rust: pin-init: add
> references to previously initialized fields"). Before then, packed
> structs compiled without any issues with pin-init and thus all prior
> kernel versions with pin-init that do not contain that commit are
> affected.
> 
> We introduced pin-init in 90e53c5e70a6 ("rust: add pin-init API core"),
> which was included in 6.4. The affected stable trees that are still
> maintained are: 6.17, 6.16, 6.12, and 6.6. Note that 6.18 and 6.19
> already contain 42415d163e5d, so they are unaffected.
> 
> I will prepare a separate patch series to backport 42415d163e5d to each
> of the affected trees, including the second patch of this series that
> documents the fact that field accessors are load-bearing for soundness.
> 
> @asahi folks, let me know if I should prioritize a solution for packed
> structs. Otherwise I'd like not support it at the moment, as that
> requires some deeper changes to the internals of pin-init. I'm tracking
> the status of packed structs in:

I have worked around this in the downstream AOP audio driver now. I did
not do that initially since it looked more involved and we were planning
to bring support back.
These structs describe messages for communication with a coprocessor via
shared memory. They are derived by observing messages by tracing. So
there is only limited understanding how the messages are formated. Their
layout has for obvious reason match exactly so `#[repr(C, packed)]` is
the obvious choice.
One of the structs had a size of N * 4 - 1 which results in a alignment
of 1. Fortunately the struct could be padded to multiple of 4.
Nevertheless it was required to replace a few u32 with an unaligned
version.

I'm not sure if there is a need to support unaligned fields in pin-init.
The workarounds in the asahi GPU and AOP audio drivers are acceptable
and could stay indefinitely.

Janne

>     https://github.com/Rust-for-Linux/pin-init/issues/112

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] rust: pin-init: internal: init: remove `#[disable_initialized_field_access]`
  2026-03-01 17:12 ` Janne Grunau
@ 2026-03-02 14:11   ` Benno Lossin
  0 siblings, 0 replies; 10+ messages in thread
From: Benno Lossin @ 2026-03-02 14:11 UTC (permalink / raw)
  To: Janne Grunau
  Cc: Gary Guo, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	asahi, rust-for-linux, linux-kernel

On Sun Mar 1, 2026 at 6:12 PM CET, Janne Grunau wrote:
> On Sat, Feb 28, 2026 at 12:37:04PM +0100, Benno Lossin wrote:
>> @asahi folks, let me know if I should prioritize a solution for packed
>> structs. Otherwise I'd like not support it at the moment, as that
>> requires some deeper changes to the internals of pin-init. I'm tracking
>> the status of packed structs in:
>
> I have worked around this in the downstream AOP audio driver now. I did
> not do that initially since it looked more involved and we were planning
> to bring support back.
> These structs describe messages for communication with a coprocessor via
> shared memory. They are derived by observing messages by tracing. So
> there is only limited understanding how the messages are formated. Their
> layout has for obvious reason match exactly so `#[repr(C, packed)]` is
> the obvious choice.
> One of the structs had a size of N * 4 - 1 which results in a alignment
> of 1. Fortunately the struct could be padded to multiple of 4.
> Nevertheless it was required to replace a few u32 with an unaligned
> version.
>
> I'm not sure if there is a need to support unaligned fields in pin-init.
> The workarounds in the asahi GPU and AOP audio drivers are acceptable
> and could stay indefinitely.

Thanks for the quick work! Yeah that sounds like a good solution for the
problem. Adding unaligned pointer support to initialization is quite a
pain. If it comes up we can still do it of course (but I'd only do it if
we can't find a workaround).

Cheers,
Benno

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-03-02 14:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-28 11:37 [PATCH 1/2] rust: pin-init: internal: init: remove `#[disable_initialized_field_access]` Benno Lossin
2026-02-28 11:37 ` [PATCH 2/2] rust: pin-init: internal: init: document load-bearing fact of field accessors Benno Lossin
2026-02-28 11:55   ` Gary Guo
2026-02-28 14:56     ` Benno Lossin
2026-02-28 14:11   ` Miguel Ojeda
2026-02-28 14:49     ` Benno Lossin
2026-02-28 12:03 ` [PATCH 1/2] rust: pin-init: internal: init: remove `#[disable_initialized_field_access]` Gary Guo
2026-02-28 14:09 ` Miguel Ojeda
2026-03-01 17:12 ` Janne Grunau
2026-03-02 14:11   ` Benno Lossin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox