[PATCH v3 00/11] `Zeroable` improvements & bindings integration

[PATCH v3 00/11] `Zeroable` improvements & bindings integration

[Benno Lossin]

This came from a discussion at [1]. And I added some more useful parts
to the original idea.

I'm not sure on the impact of build times and rust-analyzer. We're
adding a derive macro to every struct and union emitted by bindgen.
Building using my test-config took 27.7s before and 28.2s after this
change, but those are only two runs on my machine with a very reduced
config (that enables all Rust code that we have at the moment).

Maybe we have to reevaluate this when more C code is scanned by bindgen.

[1]: https://rust-for-linux.zulipchat.com/#narrow/channel/291565-Help/topic/Zeroable.20trait.20for.20C.20structs/with/509711564

Changelog
=========
* v3:
  - removed already applied patches
  - added more patches replacing usage of `mem::zeroed` and
    `MaybeUninit::zeroed().assume_init()` with the new
    `pin_init::zeroed`
  - fix rusttest target
* v2:
  - added patches with more extensive pin-init changes to `Zeroable` &
    related methods. for the upstream PR, see
    https://github.com/Rust-for-Linux/pin-init/pull/56
  - added patches replacing usage of `mem::zeroed` and
    `MaybeUninit::zeroed().assume_init()` with the new
    `pin_init::zeroed`
  - fix rust-analyzer support 
  - use import in `{uapi,bindings}/lib.rs` to avoid the `pin_init::`
    prefix
* v1: https://lore.kernel.org/all/20250520192307.259142-1-lossin@kernel.org

Benno Lossin (11):
  rust: add `pin-init` as a dependency to `bindings` and `uapi`
  rust: derive `Zeroable` for all structs & unions generated by bindgen
    where possible
  rust: miscdevice: replace `MaybeUninit::zeroed().assume_init()` with
    `pin_init::zeroed()`
  rust: phy: replace `MaybeUninit::zeroed().assume_init()` with
    `pin_init::zeroed()`
  rust: block: replace `core::mem::zeroed` with `pin_init::zeroed`
  rust: of: replace `core::mem::zeroed` with `pin_init::zeroed`
  rust: security: replace `core::mem::zeroed` with `pin_init::zeroed`
  rust: drm: replace `core::mem::zeroed` with `pin_init::zeroed`
  rust: auxiliary: replace `core::mem::zeroed` with `pin_init::zeroed`
  rust: acpi: replace `core::mem::zeroed` with `pin_init::zeroed`
  rust: cpufreq: replace `MaybeUninit::zeroed().assume_init()` with
    `pin_init::zeroed()`

 rust/Makefile                     | 14 ++++++++------
 rust/bindgen_parameters           |  4 ++++
 rust/bindings/lib.rs              |  8 ++++++++
 rust/kernel/acpi.rs               |  4 +---
 rust/kernel/auxiliary.rs          |  6 +-----
 rust/kernel/block/mq/gen_disk.rs  |  3 +--
 rust/kernel/block/mq/tag_set.rs   |  4 +---
 rust/kernel/cpufreq.rs            |  4 +---
 rust/kernel/drm/gem/mod.rs        |  4 +---
 rust/kernel/miscdevice.rs         |  8 +++-----
 rust/kernel/net/phy.rs            |  4 +---
 rust/kernel/of.rs                 |  4 +---
 rust/kernel/security.rs           |  3 +--
 rust/uapi/lib.rs                  |  2 ++
 scripts/generate_rust_analyzer.py |  4 ++--
 15 files changed, 36 insertions(+), 40 deletions(-)


base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
-- 
2.50.1

[PATCH v3 01/11] rust: add `pin-init` as a dependency to `bindings` and `uapi`

[Benno Lossin]

This allows `bindings` and `uapi` to implement `Zeroable` and use other
items from pin-init.

Co-developed-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://rust-for-linux.zulipchat.com/#narrow/channel/291565-Help/topic/Zeroable.20trait.20for.20C.20structs/near/510264158
Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/Makefile                     | 14 ++++++++------
 scripts/generate_rust_analyzer.py |  4 ++--
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/rust/Makefile b/rust/Makefile
index 4263462b8470..09d4f389240a 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -184,12 +184,12 @@ rusttestlib-kernel: $(src)/kernel/lib.rs rusttestlib-bindings rusttestlib-uapi \
     $(obj)/bindings.o FORCE
 	+$(call if_changed,rustc_test_library)
 
-rusttestlib-bindings: private rustc_target_flags = --extern ffi
-rusttestlib-bindings: $(src)/bindings/lib.rs rusttestlib-ffi FORCE
+rusttestlib-bindings: private rustc_target_flags = --extern ffi --extern pin_init
+rusttestlib-bindings: $(src)/bindings/lib.rs rusttestlib-ffi rusttestlib-pin_init FORCE
 	+$(call if_changed,rustc_test_library)
 
-rusttestlib-uapi: private rustc_target_flags = --extern ffi
-rusttestlib-uapi: $(src)/uapi/lib.rs rusttestlib-ffi FORCE
+rusttestlib-uapi: private rustc_target_flags = --extern ffi --extern pin_init
+rusttestlib-uapi: $(src)/uapi/lib.rs rusttestlib-ffi rusttestlib-pin_init FORCE
 	+$(call if_changed,rustc_test_library)
 
 quiet_cmd_rustdoc_test = RUSTDOC T $<
@@ -521,17 +521,19 @@ $(obj)/ffi.o: private skip_gendwarfksyms = 1
 $(obj)/ffi.o: $(src)/ffi.rs $(obj)/compiler_builtins.o FORCE
 	+$(call if_changed_rule,rustc_library)
 
-$(obj)/bindings.o: private rustc_target_flags = --extern ffi
+$(obj)/bindings.o: private rustc_target_flags = --extern ffi --extern pin_init
 $(obj)/bindings.o: $(src)/bindings/lib.rs \
     $(obj)/ffi.o \
+    $(obj)/pin_init.o \
     $(obj)/bindings/bindings_generated.rs \
     $(obj)/bindings/bindings_helpers_generated.rs FORCE
 	+$(call if_changed_rule,rustc_library)
 
-$(obj)/uapi.o: private rustc_target_flags = --extern ffi
+$(obj)/uapi.o: private rustc_target_flags = --extern ffi --extern pin_init
 $(obj)/uapi.o: private skip_gendwarfksyms = 1
 $(obj)/uapi.o: $(src)/uapi/lib.rs \
     $(obj)/ffi.o \
+    $(obj)/pin_init.o \
     $(obj)/uapi/uapi_generated.rs FORCE
 	+$(call if_changed_rule,rustc_library)
 
diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index 7c3ea2b55041..fc27f0cca752 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -139,8 +139,8 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs, core_edit
             "exclude_dirs": [],
         }
 
-    append_crate_with_generated("bindings", ["core", "ffi"])
-    append_crate_with_generated("uapi", ["core", "ffi"])
+    append_crate_with_generated("bindings", ["core", "ffi", "pin_init"])
+    append_crate_with_generated("uapi", ["core", "ffi", "pin_init"])
     append_crate_with_generated("kernel", ["core", "macros", "build_error", "pin_init", "ffi", "bindings", "uapi"])
 
     def is_root_crate(build_file, target):
-- 
2.50.1

[PATCH v3 02/11] rust: derive `Zeroable` for all structs & unions generated by bindgen where possible

[Benno Lossin]

Using the `--with-derive-custom-{struct,union}` option of bindgen, add
`#[derive(MaybeZeroable)]` to every struct & union. This makes those
types implement `Zeroable` if all their fields implement it.

Sadly bindgen doesn't add custom derives to the `__BindgenBitfieldUnit`
struct. So manually implement `Zeroable` for that.

Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/bindgen_parameters | 4 ++++
 rust/bindings/lib.rs    | 8 ++++++++
 rust/uapi/lib.rs        | 2 ++
 3 files changed, 14 insertions(+)

diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
index 0f96af8b9a7f..307545cf7363 100644
--- a/rust/bindgen_parameters
+++ b/rust/bindgen_parameters
@@ -34,3 +34,7 @@
 # We use const helpers to aid bindgen, to avoid conflicts when constants are
 # recognized, block generation of the non-helper constants.
 --blocklist-item ARCH_SLAB_MINALIGN
+
+# Structs should implement Zeroable when all of their fields do.
+--with-derive-custom-struct .*=MaybeZeroable
+--with-derive-custom-union .*=MaybeZeroable
diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
index 474cc98c48a3..0c57cf9b4004 100644
--- a/rust/bindings/lib.rs
+++ b/rust/bindings/lib.rs
@@ -31,11 +31,19 @@
 #[allow(clippy::undocumented_unsafe_blocks)]
 #[cfg_attr(CONFIG_RUSTC_HAS_UNNECESSARY_TRANSMUTES, allow(unnecessary_transmutes))]
 mod bindings_raw {
+    use pin_init::{MaybeZeroable, Zeroable};
+
     // Manual definition for blocklisted types.
     type __kernel_size_t = usize;
     type __kernel_ssize_t = isize;
     type __kernel_ptrdiff_t = isize;
 
+    // `bindgen` doesn't automatically do this, see
+    // <https://github.com/rust-lang/rust-bindgen/issues/3196>
+    //
+    // SAFETY: `__BindgenBitfieldUnit<Storage>` is a newtype around `Storage`.
+    unsafe impl<Storage> Zeroable for __BindgenBitfieldUnit<Storage> where Storage: Zeroable {}
+
     // Use glob import here to expose all helpers.
     // Symbols defined within the module will take precedence to the glob import.
     pub use super::bindings_helper::*;
diff --git a/rust/uapi/lib.rs b/rust/uapi/lib.rs
index 31c2f713313f..1d5fd9efb93e 100644
--- a/rust/uapi/lib.rs
+++ b/rust/uapi/lib.rs
@@ -34,4 +34,6 @@
 type __kernel_ssize_t = isize;
 type __kernel_ptrdiff_t = isize;
 
+use pin_init::MaybeZeroable;
+
 include!(concat!(env!("OBJTREE"), "/rust/uapi/uapi_generated.rs"));
-- 
2.50.1

[PATCH v3 03/11] rust: miscdevice: replace `MaybeUninit::zeroed().assume_init()` with `pin_init::zeroed()`

[Benno Lossin]

All types in `bindings` implement `Zeroable` if they can, so use
`pin_init::zeroed` instead of relying on `unsafe` code.

If this ends up not compiling in the future, something in bindgen or on
the C side changed and is most likely incorrect.

Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/kernel/miscdevice.rs | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index 6373fe183b27..cecd1f265324 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -19,7 +19,7 @@
     seq_file::SeqFile,
     types::{ForeignOwnable, Opaque},
 };
-use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin};
+use core::{marker::PhantomData, pin::Pin};
 
 /// Options for creating a misc device.
 #[derive(Copy, Clone)]
@@ -31,8 +31,7 @@ pub struct MiscDeviceOptions {
 impl MiscDeviceOptions {
     /// Create a raw `struct miscdev` ready for registration.
     pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
-        // SAFETY: All zeros is valid for this C type.
-        let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() };
+        let mut result: bindings::miscdevice = pin_init::zeroed();
         result.minor = bindings::MISC_DYNAMIC_MINOR as ffi::c_int;
         result.name = self.name.as_char_ptr();
         result.fops = MiscdeviceVTable::<T>::build();
@@ -359,8 +358,7 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
         } else {
             None
         },
-        // SAFETY: All zeros is a valid value for `bindings::file_operations`.
-        ..unsafe { MaybeUninit::zeroed().assume_init() }
+        ..pin_init::zeroed()
     };
 
     const fn build() -> &'static bindings::file_operations {
-- 
2.50.1

[PATCH v3 04/11] rust: phy: replace `MaybeUninit::zeroed().assume_init()` with `pin_init::zeroed()`

[Benno Lossin]

All types in `bindings` implement `Zeroable` if they can, so use
`pin_init::zeroed` instead of relying on `unsafe` code.

If this ends up not compiling in the future, something in bindgen or on
the C side changed and is most likely incorrect.

Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/kernel/net/phy.rs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 7de5cc7a0eee..3e5565a6a130 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -556,9 +556,7 @@ pub const fn create_phy_driver<T: Driver>() -> DriverVTable {
         } else {
             None
         },
-        // SAFETY: The rest is zeroed out to initialize `struct phy_driver`,
-        // sets `Option<&F>` to be `None`.
-        ..unsafe { core::mem::MaybeUninit::<bindings::phy_driver>::zeroed().assume_init() }
+        ..pin_init::zeroed()
     }))
 }
 
-- 
2.50.1

[PATCH v3 05/11] rust: block: replace `core::mem::zeroed` with `pin_init::zeroed`

[Benno Lossin]

All types in `bindings` implement `Zeroable` if they can, so use
`pin_init::zeroed` instead of relying on `unsafe` code.

If this ends up not compiling in the future, something in bindgen or on
the C side changed and is most likely incorrect.

Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/kernel/block/mq/gen_disk.rs | 3 +--
 rust/kernel/block/mq/tag_set.rs  | 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index cd54cd64ea88..75b90fe20c7d 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -93,8 +93,7 @@ pub fn build<T: Operations>(
         name: fmt::Arguments<'_>,
         tagset: Arc<TagSet<T>>,
     ) -> Result<GenDisk<T>> {
-        // SAFETY: `bindings::queue_limits` contain only fields that are valid when zeroed.
-        let mut lim: bindings::queue_limits = unsafe { core::mem::zeroed() };
+        let mut lim: bindings::queue_limits = pin_init::zeroed();
 
         lim.logical_block_size = self.logical_block_size;
         lim.physical_block_size = self.physical_block_size;
diff --git a/rust/kernel/block/mq/tag_set.rs b/rust/kernel/block/mq/tag_set.rs
index c3cf56d52bee..dae9df408a86 100644
--- a/rust/kernel/block/mq/tag_set.rs
+++ b/rust/kernel/block/mq/tag_set.rs
@@ -38,9 +38,7 @@ pub fn new(
         num_tags: u32,
         num_maps: u32,
     ) -> impl PinInit<Self, error::Error> {
-        // SAFETY: `blk_mq_tag_set` only contains integers and pointers, which
-        // all are allowed to be 0.
-        let tag_set: bindings::blk_mq_tag_set = unsafe { core::mem::zeroed() };
+        let tag_set: bindings::blk_mq_tag_set = pin_init::zeroed();
         let tag_set: Result<_> = core::mem::size_of::<RequestDataWrapper>()
             .try_into()
             .map(|cmd_size| {
-- 
2.50.1

[PATCH v3 06/11] rust: of: replace `core::mem::zeroed` with `pin_init::zeroed`

[Benno Lossin]

All types in `bindings` implement `Zeroable` if they can, so use
`pin_init::zeroed` instead of relying on `unsafe` code.

If this ends up not compiling in the future, something in bindgen or on
the C side changed and is most likely incorrect.

Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/kernel/of.rs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index b76b35265df2..6230bbf2ebc1 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -35,9 +35,7 @@ impl DeviceId {
     /// Create a new device id from an OF 'compatible' string.
     pub const fn new(compatible: &'static CStr) -> Self {
         let src = compatible.as_bytes_with_nul();
-        // Replace with `bindings::of_device_id::default()` once stabilized for `const`.
-        // SAFETY: FFI type is valid to be zero-initialized.
-        let mut of: bindings::of_device_id = unsafe { core::mem::zeroed() };
+        let mut of: bindings::of_device_id = pin_init::zeroed();
 
         // TODO: Use `copy_from_slice` once stabilized for `const`.
         let mut i = 0;
-- 
2.50.1

[PATCH v3 07/11] rust: security: replace `core::mem::zeroed` with `pin_init::zeroed`

[Benno Lossin]

All types in `bindings` implement `Zeroable` if they can, so use
`pin_init::zeroed` instead of relying on `unsafe` code.

If this ends up not compiling in the future, something in bindgen or on
the C side changed and is most likely incorrect.

Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/kernel/security.rs | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/rust/kernel/security.rs b/rust/kernel/security.rs
index 0c63e9e7e564..11a6493525f3 100644
--- a/rust/kernel/security.rs
+++ b/rust/kernel/security.rs
@@ -25,8 +25,7 @@ impl SecurityCtx {
     /// Get the security context given its id.
     #[inline]
     pub fn from_secid(secid: u32) -> Result<Self> {
-        // SAFETY: `struct lsm_context` can be initialized to all zeros.
-        let mut ctx: bindings::lsm_context = unsafe { core::mem::zeroed() };
+        let mut ctx: bindings::lsm_context = pin_init::zeroed();
 
         // SAFETY: Just a C FFI call. The pointer is valid for writes.
         to_result(unsafe { bindings::security_secid_to_secctx(secid, &mut ctx) })?;
-- 
2.50.1

[PATCH v3 08/11] rust: drm: replace `core::mem::zeroed` with `pin_init::zeroed`

[Benno Lossin]

All types in `bindings` implement `Zeroable` if they can, so use
`pin_init::zeroed` instead of relying on `unsafe` code.

If this ends up not compiling in the future, something in bindgen or on
the C side changed and is most likely incorrect.

Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/kernel/drm/gem/mod.rs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index b71821cfb5ea..6532513090de 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -308,9 +308,7 @@ impl<T: DriverObject> AllocImpl for Object<T> {
 }
 
 pub(super) const fn create_fops() -> bindings::file_operations {
-    // SAFETY: As by the type invariant, it is safe to initialize `bindings::file_operations`
-    // zeroed.
-    let mut fops: bindings::file_operations = unsafe { core::mem::zeroed() };
+    let mut fops: bindings::file_operations = pin_init::zeroed();
 
     fops.owner = core::ptr::null_mut();
     fops.open = Some(bindings::drm_open);
-- 
2.50.1

[PATCH v3 09/11] rust: auxiliary: replace `core::mem::zeroed` with `pin_init::zeroed`

[Benno Lossin]

All types in `bindings` implement `Zeroable` if they can, so use
`pin_init::zeroed` instead of relying on `unsafe` code.

If this ends up not compiling in the future, something in bindgen or on
the C side changed and is most likely incorrect.

Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/kernel/auxiliary.rs | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index 4749fb6bffef..10ec1f3b4a6d 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -108,11 +108,7 @@ pub const fn new(modname: &'static CStr, name: &'static CStr) -> Self {
         let name = name.as_bytes_with_nul();
         let modname = modname.as_bytes_with_nul();
 
-        // TODO: Replace with `bindings::auxiliary_device_id::default()` once stabilized for
-        // `const`.
-        //
-        // SAFETY: FFI type is valid to be zero-initialized.
-        let mut id: bindings::auxiliary_device_id = unsafe { core::mem::zeroed() };
+        let mut id: bindings::auxiliary_device_id = pin_init::zeroed();
 
         let mut i = 0;
         while i < modname.len() {
-- 
2.50.1

[PATCH v3 10/11] rust: acpi: replace `core::mem::zeroed` with `pin_init::zeroed`

[Benno Lossin]

All types in `bindings` implement `Zeroable` if they can, so use
`pin_init::zeroed` instead of relying on `unsafe` code.

If this ends up not compiling in the future, something in bindgen or on
the C side changed and is most likely incorrect.

Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/kernel/acpi.rs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
index 7ae317368b00..f9488be9249c 100644
--- a/rust/kernel/acpi.rs
+++ b/rust/kernel/acpi.rs
@@ -42,9 +42,7 @@ pub const fn new(id: &'static CStr) -> Self {
             "ID exceeds 16 bytes"
         );
         let src = id.as_bytes_with_nul();
-        // Replace with `bindings::acpi_device_id::default()` once stabilized for `const`.
-        // SAFETY: FFI type is valid to be zero-initialized.
-        let mut acpi: bindings::acpi_device_id = unsafe { core::mem::zeroed() };
+        let mut acpi: bindings::acpi_device_id = pin_init::zeroed();
         let mut i = 0;
         while i < src.len() {
             acpi.id[i] = src[i];
-- 
2.50.1

[PATCH v3 11/11] rust: cpufreq: replace `MaybeUninit::zeroed().assume_init()` with `pin_init::zeroed()`

[Benno Lossin]

All types in `bindings` implement `Zeroable` if they can, so use
`pin_init::zeroed` instead of relying on `unsafe` code.

If this ends up not compiling in the future, something in bindgen or on
the C side changed and is most likely incorrect.

Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/kernel/cpufreq.rs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index afc15e72a7c3..be2dffbdb546 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -27,7 +27,6 @@
 use core::{
     cell::UnsafeCell,
     marker::PhantomData,
-    mem::MaybeUninit,
     ops::{Deref, DerefMut},
     pin::Pin,
     ptr,
@@ -1013,8 +1012,7 @@ impl<T: Driver> Registration<T> {
         } else {
             None
         },
-        // SAFETY: All zeros is a valid value for `bindings::cpufreq_driver`.
-        ..unsafe { MaybeUninit::zeroed().assume_init() }
+        ..pin_init::zeroed()
     };
 
     const fn copy_name(name: &'static CStr) -> [c_char; CPUFREQ_NAME_LEN] {
-- 
2.50.1

Re: [PATCH v3 11/11] rust: cpufreq: replace `MaybeUninit::zeroed().assume_init()` with `pin_init::zeroed()`

[Viresh Kumar]

On 14-08-25, 11:30, Benno Lossin wrote:
> All types in `bindings` implement `Zeroable` if they can, so use
> `pin_init::zeroed` instead of relying on `unsafe` code.
> 
> If this ends up not compiling in the future, something in bindgen or on
> the C side changed and is most likely incorrect.
> 
> Signed-off-by: Benno Lossin <lossin@kernel.org>
> ---
>  rust/kernel/cpufreq.rs | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v3 07/11] rust: security: replace `core::mem::zeroed` with `pin_init::zeroed`

[Paul Moore]

On Thu, Aug 14, 2025 at 5:31 AM Benno Lossin <lossin@kernel.org> wrote:
>
> All types in `bindings` implement `Zeroable` if they can, so use
> `pin_init::zeroed` instead of relying on `unsafe` code.
>
> If this ends up not compiling in the future, something in bindgen or on
> the C side changed and is most likely incorrect.
>
> Signed-off-by: Benno Lossin <lossin@kernel.org>
> ---
>  rust/kernel/security.rs | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

I'm happy to take this via the LSM tree, but it would be nice to see a
Reviewed-by/Acked-by from someone with a better understanding of Rust
:)

Alice, you've reviewed/submitted LSM rust bits in the past, can you
look at this?

> diff --git a/rust/kernel/security.rs b/rust/kernel/security.rs
> index 0c63e9e7e564..11a6493525f3 100644
> --- a/rust/kernel/security.rs
> +++ b/rust/kernel/security.rs
> @@ -25,8 +25,7 @@ impl SecurityCtx {
>      /// Get the security context given its id.
>      #[inline]
>      pub fn from_secid(secid: u32) -> Result<Self> {
> -        // SAFETY: `struct lsm_context` can be initialized to all zeros.
> -        let mut ctx: bindings::lsm_context = unsafe { core::mem::zeroed() };
> +        let mut ctx: bindings::lsm_context = pin_init::zeroed();
>
>          // SAFETY: Just a C FFI call. The pointer is valid for writes.
>          to_result(unsafe { bindings::security_secid_to_secctx(secid, &mut ctx) })?;
> --
> 2.50.1

-- 
paul-moore.com

Re: [PATCH v3 07/11] rust: security: replace `core::mem::zeroed` with `pin_init::zeroed`

[Miguel Ojeda]

On Thu, Aug 14, 2025 at 5:19 PM Paul Moore <paul@paul-moore.com> wrote:
>
> I'm happy to take this via the LSM tree, but it would be nice to see a
> Reviewed-by/Acked-by from someone with a better understanding of Rust
> :)

I think the idea is to take all these through the Rust one with
Acked-bys from the maintainers (or we can skip this one and do it in a
future cycle when the first patches get in).

In any case, Benno is very knowledgeable in Rust -- he wrote the
library being called here -- so unless you see something out of the
ordinary, it seems OK to me.

Thanks!

Cheers,
Miguel

Re: [PATCH v3 07/11] rust: security: replace `core::mem::zeroed` with `pin_init::zeroed`

[Miguel Ojeda]

On Thu, Aug 14, 2025 at 5:31 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> I think the idea is to take all these through the Rust one with

Or through the pin-init one, which gets pulled into the Rust one.

Cheers,
Miguel

Re: [PATCH v3 07/11] rust: security: replace `core::mem::zeroed` with `pin_init::zeroed`

[Benno Lossin]

On Thu Aug 14, 2025 at 5:33 PM CEST, Miguel Ojeda wrote:
> On Thu, Aug 14, 2025 at 5:31 PM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
>>
>> I think the idea is to take all these through the Rust one with
>
> Or through the pin-init one, which gets pulled into the Rust one.

There are no pin-init changes in this patchset (in v2 there were, but I
took them for v6.16), so I think it should go via the rust tree.

We can of course wait with patches 3-11 until the next cycle and let
maintainers pick them individually, but since they are so small, I think
it's easier to just pick them all at once via the rust tree.

---
Cheers,
Benno

Re: [PATCH v3 07/11] rust: security: replace `core::mem::zeroed` with `pin_init::zeroed`

[Paul Moore]

On Thu, Aug 14, 2025 at 11:31 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
> On Thu, Aug 14, 2025 at 5:19 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > I'm happy to take this via the LSM tree, but it would be nice to see a
> > Reviewed-by/Acked-by from someone with a better understanding of Rust
> > :)
>
> I think the idea is to take all these through the Rust one with
> Acked-bys from the maintainers (or we can skip this one and do it in a
> future cycle when the first patches get in).

[CC'd the LSM list, as I just realized it wasn't on the original patch
posting; in the future please include the LSM list on LSM related Rust
patchsets/patches]

That's fine, it wasn't clear from the post that was the plan, and I
vaguely recalled from past conversations with Rust devs that they
preferred Rust wrappers/helpers to go in via the associated subsystem
tree.

> In any case, Benno is very knowledgeable in Rust -- he wrote the
> library being called here -- so unless you see something out of the
> ordinary, it seems OK to me.

My comment asking for additional review/ACK tags wasn't due to any
distrust of Benno - thank you for your work Benno - it is just a
matter of trying to make sure there are at least two sets of
(knowledgeable) eyes on a patch before it is merged.  If it is
something I'm merging into one the trees I maintain, normally I count
myself as the second set of eyes, but in this case I don't (yet)
consider myself a knowledgeable Rust reviewer so I was asking for an
additional explicit review tag.  If someone else is going to merge
this patch{set}, then it's up to them; although I would hope they
would do something similar.

-- 
paul-moore.com

Re: [PATCH v3 07/11] rust: security: replace `core::mem::zeroed` with `pin_init::zeroed`

[Benno Lossin]

On Thu Aug 14, 2025 at 5:54 PM CEST, Paul Moore wrote:
> On Thu, Aug 14, 2025 at 11:31 AM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
>> On Thu, Aug 14, 2025 at 5:19 PM Paul Moore <paul@paul-moore.com> wrote:
>> >
>> > I'm happy to take this via the LSM tree, but it would be nice to see a
>> > Reviewed-by/Acked-by from someone with a better understanding of Rust
>> > :)
>>
>> I think the idea is to take all these through the Rust one with
>> Acked-bys from the maintainers (or we can skip this one and do it in a
>> future cycle when the first patches get in).
>
> [CC'd the LSM list, as I just realized it wasn't on the original patch
> posting; in the future please include the LSM list on LSM related Rust
> patchsets/patches]

I checked and I didn't find a maintainers entry for that this file & the
LSM list. I'm using scripts/get_maintainer.pl to get the people I send
patches to and that also checks git commits, so I guess it added you
through that (which is very good :). So can we add a maintainers entry
for `rust/kernel/security.rs` so people don't miss this in the future?
Thanks!

---
Cheers,
Benno

Re: [PATCH v3 07/11] rust: security: replace `core::mem::zeroed` with `pin_init::zeroed`

[Miguel Ojeda]

On Thu, Aug 14, 2025 at 5:38 PM Benno Lossin <lossin@kernel.org> wrote:
>
> There are no pin-init changes in this patchset (in v2 there were, but I
> took them for v6.16), so I think it should go via the rust tree.

Sounds good.

> We can of course wait with patches 3-11 until the next cycle and let
> maintainers pick them individually, but since they are so small, I think
> it's easier to just pick them all at once via the rust tree.

Yeah, either way is fine. I would say let's just land the ones we get
Acked-bys for, even if they are not all.

Cheers,
Miguel

Re: [PATCH v3 07/11] rust: security: replace `core::mem::zeroed` with `pin_init::zeroed`

[Miguel Ojeda]

On Thu, Aug 14, 2025 at 5:54 PM Paul Moore <paul@paul-moore.com> wrote:
>
> That's fine, it wasn't clear from the post that was the plan, and I
> vaguely recalled from past conversations with Rust devs that they
> preferred Rust wrappers/helpers to go in via the associated subsystem
> tree.

Yeah, it is still the case that Rust abstractions and Rust code in
general should land through the best tree possible.

However, sometimes tree-wide improvements may be easy to do with
simple Acked-bys.

> My comment asking for additional review/ACK tags wasn't due to any
> distrust of Benno - thank you for your work Benno - it is just a

Ah, sorry, I didn't mean that you distrusted Benno or anything like
that. I was trying to give context in case it helped you evaluate the
patch/risk, which also allowed me at the same time to acknowledge
Benno's experience/work.

We were mainly expecting the Acked-bys to proceed with the cleanups
here to remove these unsafe blocks, but we definitely want more tags
if possible in all patches, as always (by the time this gets applied,
we will hopefully get some more).

Cheers,
Miguel

Re: [PATCH v3 07/11] rust: security: replace `core::mem::zeroed` with `pin_init::zeroed`

[Paul Moore]

On Thu, Aug 14, 2025 at 1:28 PM Benno Lossin <lossin@kernel.org> wrote:
> On Thu Aug 14, 2025 at 5:54 PM CEST, Paul Moore wrote:
> > On Thu, Aug 14, 2025 at 11:31 AM Miguel Ojeda
> > <miguel.ojeda.sandonis@gmail.com> wrote:
> >> On Thu, Aug 14, 2025 at 5:19 PM Paul Moore <paul@paul-moore.com> wrote:
> >> >
> >> > I'm happy to take this via the LSM tree, but it would be nice to see a
> >> > Reviewed-by/Acked-by from someone with a better understanding of Rust
> >> > :)
> >>
> >> I think the idea is to take all these through the Rust one with
> >> Acked-bys from the maintainers (or we can skip this one and do it in a
> >> future cycle when the first patches get in).
> >
> > [CC'd the LSM list, as I just realized it wasn't on the original patch
> > posting; in the future please include the LSM list on LSM related Rust
> > patchsets/patches]
>
> I checked and I didn't find a maintainers entry for that this file & the
> LSM list. I'm using scripts/get_maintainer.pl to get the people I send
> patches to and that also checks git commits, so I guess it added you
> through that (which is very good :). So can we add a maintainers entry
> for `rust/kernel/security.rs` so people don't miss this in the future?
> Thanks!

Here ya go ...

https://lore.kernel.org/linux-security-module/20250814215952.238316-2-paul@paul-moore.com/

-- 
paul-moore.com

Re: [PATCH v3 00/11] `Zeroable` improvements & bindings integration

[Alice Ryhl]

On Thu, Aug 14, 2025 at 11:31 AM Benno Lossin <lossin@kernel.org> wrote:
>
> This came from a discussion at [1]. And I added some more useful parts
> to the original idea.
>
> I'm not sure on the impact of build times and rust-analyzer. We're
> adding a derive macro to every struct and union emitted by bindgen.
> Building using my test-config took 27.7s before and 28.2s after this
> change, but those are only two runs on my machine with a very reduced
> config (that enables all Rust code that we have at the moment).
>
> Maybe we have to reevaluate this when more C code is scanned by bindgen.

Imo we should re-export Zeroable from the ffi crate and use it via
kernel::ffi in most cases. I don't really want to see pin_init here:

let mut ctx: bindings::lsm_context = pin_init::zeroed();

Alice

Re: [PATCH v3 01/11] rust: add `pin-init` as a dependency to `bindings` and `uapi`

[Alice Ryhl]

On Thu, Aug 14, 2025 at 11:31 AM Benno Lossin <lossin@kernel.org> wrote:
>
> This allows `bindings` and `uapi` to implement `Zeroable` and use other
> items from pin-init.
>
> Co-developed-by: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/291565-Help/topic/Zeroable.20trait.20for.20C.20structs/near/510264158
> Signed-off-by: Benno Lossin <lossin@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

>  rust/Makefile                     | 14 ++++++++------
>  scripts/generate_rust_analyzer.py |  4 ++--
>  2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/rust/Makefile b/rust/Makefile
> index 4263462b8470..09d4f389240a 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -184,12 +184,12 @@ rusttestlib-kernel: $(src)/kernel/lib.rs rusttestlib-bindings rusttestlib-uapi \
>      $(obj)/bindings.o FORCE
>         +$(call if_changed,rustc_test_library)
>
> -rusttestlib-bindings: private rustc_target_flags = --extern ffi
> -rusttestlib-bindings: $(src)/bindings/lib.rs rusttestlib-ffi FORCE
> +rusttestlib-bindings: private rustc_target_flags = --extern ffi --extern pin_init
> +rusttestlib-bindings: $(src)/bindings/lib.rs rusttestlib-ffi rusttestlib-pin_init FORCE
>         +$(call if_changed,rustc_test_library)
>
> -rusttestlib-uapi: private rustc_target_flags = --extern ffi
> -rusttestlib-uapi: $(src)/uapi/lib.rs rusttestlib-ffi FORCE
> +rusttestlib-uapi: private rustc_target_flags = --extern ffi --extern pin_init
> +rusttestlib-uapi: $(src)/uapi/lib.rs rusttestlib-ffi rusttestlib-pin_init FORCE
>         +$(call if_changed,rustc_test_library)
>
>  quiet_cmd_rustdoc_test = RUSTDOC T $<
> @@ -521,17 +521,19 @@ $(obj)/ffi.o: private skip_gendwarfksyms = 1
>  $(obj)/ffi.o: $(src)/ffi.rs $(obj)/compiler_builtins.o FORCE
>         +$(call if_changed_rule,rustc_library)
>
> -$(obj)/bindings.o: private rustc_target_flags = --extern ffi
> +$(obj)/bindings.o: private rustc_target_flags = --extern ffi --extern pin_init
>  $(obj)/bindings.o: $(src)/bindings/lib.rs \
>      $(obj)/ffi.o \
> +    $(obj)/pin_init.o \
>      $(obj)/bindings/bindings_generated.rs \
>      $(obj)/bindings/bindings_helpers_generated.rs FORCE
>         +$(call if_changed_rule,rustc_library)
>
> -$(obj)/uapi.o: private rustc_target_flags = --extern ffi
> +$(obj)/uapi.o: private rustc_target_flags = --extern ffi --extern pin_init
>  $(obj)/uapi.o: private skip_gendwarfksyms = 1
>  $(obj)/uapi.o: $(src)/uapi/lib.rs \
>      $(obj)/ffi.o \
> +    $(obj)/pin_init.o \
>      $(obj)/uapi/uapi_generated.rs FORCE
>         +$(call if_changed_rule,rustc_library)
>
> diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
> index 7c3ea2b55041..fc27f0cca752 100755
> --- a/scripts/generate_rust_analyzer.py
> +++ b/scripts/generate_rust_analyzer.py
> @@ -139,8 +139,8 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs, core_edit
>              "exclude_dirs": [],
>          }
>
> -    append_crate_with_generated("bindings", ["core", "ffi"])
> -    append_crate_with_generated("uapi", ["core", "ffi"])
> +    append_crate_with_generated("bindings", ["core", "ffi", "pin_init"])
> +    append_crate_with_generated("uapi", ["core", "ffi", "pin_init"])
>      append_crate_with_generated("kernel", ["core", "macros", "build_error", "pin_init", "ffi", "bindings", "uapi"])
>
>      def is_root_crate(build_file, target):
> --
> 2.50.1
>

Re: [PATCH v3 02/11] rust: derive `Zeroable` for all structs & unions generated by bindgen where possible

[Alice Ryhl]

On Thu, Aug 14, 2025 at 11:31 AM Benno Lossin <lossin@kernel.org> wrote:
>
> Using the `--with-derive-custom-{struct,union}` option of bindgen, add
> `#[derive(MaybeZeroable)]` to every struct & union. This makes those
> types implement `Zeroable` if all their fields implement it.
>
> Sadly bindgen doesn't add custom derives to the `__BindgenBitfieldUnit`
> struct. So manually implement `Zeroable` for that.
>
> Signed-off-by: Benno Lossin <lossin@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>