mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] rust: bitfield: require Zeroable storage for Zeroable impl
@ 2026-09-13 15:34 Yilin Chen
  2026-09-13 19:37 ` Gary Guo
  0 siblings, 1 reply; 5+ messages in thread
From: Yilin Chen @ 2026-09-13 15:34 UTC (permalink / raw)
  To: rust-for-linux; +Cc: linux-kernel, acourbot, yury.norov, ojeda, Yilin Chen

The bitfield! macro implements Zeroable for generated wrapper
types. This assumes the storage type accepts an all-zero bit
pattern. However, the macro accepts any type and does not encode
that requirement.

Add a Zeroable bound to the generated implementation. This prevents
invalid storage types from obtaining an unsound Zeroable implementation.

Fixes: b7b8b4ccdad4 ("rust: extract `bitfield!` macro from `register!`")
Assisted-by: Codex:GPT-5.6 Sol
Signed-off-by: Yilin Chen <1479826151@qq.com>
---
 rust/kernel/bitfield.rs | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
index a0d0894..3dd4dea 100644
--- a/rust/kernel/bitfield.rs
+++ b/rust/kernel/bitfield.rs
@@ -330,8 +330,12 @@ macro_rules! bitfield {
             }
         }
 
-        // SAFETY: `$storage` is `Zeroable` and `$name` is transparent.
-        unsafe impl ::pin_init::Zeroable for $name {}
+        // SAFETY:
+        // - `$storage: Zeroable` guarantees that the all-zero bit pattern is valid.
+        // - `$name` is `repr(transparent)` over `$storage`.
+        unsafe impl ::pin_init::Zeroable for $name
+        where $storage: ::pin_init::Zeroable
+        {}
 
         impl ::core::convert::From<$name> for $storage {
             #[inline(always)]
-- 
2.25.1


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

* Re: [PATCH] rust: bitfield: require Zeroable storage for Zeroable impl
  2026-09-13 15:34 [PATCH] rust: bitfield: require Zeroable storage for Zeroable impl Yilin Chen
@ 2026-09-13 19:37 ` Gary Guo
  2026-09-21  6:32   ` Yilin Chen
  0 siblings, 1 reply; 5+ messages in thread
From: Gary Guo @ 2026-09-13 19:37 UTC (permalink / raw)
  To: Yilin Chen, rust-for-linux; +Cc: linux-kernel, acourbot, yury.norov, ojeda

On Sun Sep 13, 2026 at 4:34 PM BST, Yilin Chen wrote:
> The bitfield! macro implements Zeroable for generated wrapper
> types. This assumes the storage type accepts an all-zero bit
> pattern. However, the macro accepts any type and does not encode
> that requirement.

How? The bitfield macro will fail for types other than primtive integers.

Best,
Gary

>
> Add a Zeroable bound to the generated implementation. This prevents
> invalid storage types from obtaining an unsound Zeroable implementation.
>
> Fixes: b7b8b4ccdad4 ("rust: extract `bitfield!` macro from `register!`")
> Assisted-by: Codex:GPT-5.6 Sol
> Signed-off-by: Yilin Chen <1479826151@qq.com>
> ---
>  rust/kernel/bitfield.rs | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
> index a0d0894..3dd4dea 100644
> --- a/rust/kernel/bitfield.rs
> +++ b/rust/kernel/bitfield.rs
> @@ -330,8 +330,12 @@ macro_rules! bitfield {
>              }
>          }
>  
> -        // SAFETY: `$storage` is `Zeroable` and `$name` is transparent.
> -        unsafe impl ::pin_init::Zeroable for $name {}
> +        // SAFETY:
> +        // - `$storage: Zeroable` guarantees that the all-zero bit pattern is valid.
> +        // - `$name` is `repr(transparent)` over `$storage`.
> +        unsafe impl ::pin_init::Zeroable for $name
> +        where $storage: ::pin_init::Zeroable
> +        {}
>  
>          impl ::core::convert::From<$name> for $storage {
>              #[inline(always)]



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

* Re: [PATCH] rust: bitfield: require Zeroable storage for Zeroable impl
  2026-09-13 19:37 ` Gary Guo
@ 2026-09-21  6:32   ` Yilin Chen
  2026-09-21  7:18     ` Alexandre Courbot
  0 siblings, 1 reply; 5+ messages in thread
From: Yilin Chen @ 2026-09-21  6:32 UTC (permalink / raw)
  To: Gary Guo
  Cc: Yilin Chen, rust-for-linux, linux-kernel, acourbot, yury.norov, ojeda

Hi Gary,

Resending my reply for visibility on the mailing list.

You are right that a bitfield with at least one field generates a use of
`Bounded<$storage, ...>`, which requires the storage type to implement
`Integer`. However, `bitfield!` also accepts an empty field list. In that
case, no `Bounded` use is generated, so the `Integer` requirement is
absent, while the macro still generates the unconditional `Zeroable`
implementation.

For example:

    use core::num::NonZeroU32;
    use pin_init::Zeroable;

    bitfield! {
        struct Bad(NonZeroU32) {}
    }

    fn check_bad_zeroable() {
        let _: Bad = <Bad as Zeroable>::zeroed();
    }

This currently compiles. Since the all-zero bit pattern is invalid for
`NonZeroU32`, the generated `Zeroable` implementation is unsound. This is
why I think the explicit `$storage: Zeroable` bound is necessary.

Best regards,
Yilin


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

* Re: [PATCH] rust: bitfield: require Zeroable storage for Zeroable impl
  2026-09-21  6:32   ` Yilin Chen
@ 2026-09-21  7:18     ` Alexandre Courbot
  2026-09-21 10:05       ` Gary Guo
  0 siblings, 1 reply; 5+ messages in thread
From: Alexandre Courbot @ 2026-09-21  7:18 UTC (permalink / raw)
  To: Yilin Chen; +Cc: Gary Guo, rust-for-linux, linux-kernel, yury.norov, ojeda

On Mon Sep 21, 2026 at 3:32 PM JST, Yilin Chen wrote:
> Hi Gary,
>
> Resending my reply for visibility on the mailing list.
>
> You are right that a bitfield with at least one field generates a use of
> `Bounded<$storage, ...>`, which requires the storage type to implement
> `Integer`. However, `bitfield!` also accepts an empty field list. In that
> case, no `Bounded` use is generated, so the `Integer` requirement is
> absent, while the macro still generates the unconditional `Zeroable`
> implementation.
>
> For example:
>
>     use core::num::NonZeroU32;
>     use pin_init::Zeroable;
>
>     bitfield! {
>         struct Bad(NonZeroU32) {}
>     }
>
>     fn check_bad_zeroable() {
>         let _: Bad = <Bad as Zeroable>::zeroed();
>     }
>
> This currently compiles.

No it doesn't.

error[E0277]: the trait bound `core::num::NonZero<u32>: kernel::mem::AsRepr` is not satisfied
364 | /         bitfield! {
365 | |             struct Bad(NonZeroU32) {}
366 | |         }
    | |_________^ the trait `kernel::mem::AsRepr` is not implemented for `core::num::NonZero<u32>`

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

* Re: [PATCH] rust: bitfield: require Zeroable storage for Zeroable impl
  2026-09-21  7:18     ` Alexandre Courbot
@ 2026-09-21 10:05       ` Gary Guo
  0 siblings, 0 replies; 5+ messages in thread
From: Gary Guo @ 2026-09-21 10:05 UTC (permalink / raw)
  To: Alexandre Courbot, Yilin Chen
  Cc: Gary Guo, rust-for-linux, linux-kernel, yury.norov, ojeda

On Mon Sep 21, 2026 at 8:18 AM BST, Alexandre Courbot wrote:
> On Mon Sep 21, 2026 at 3:32 PM JST, Yilin Chen wrote:
>> Hi Gary,
>>
>> Resending my reply for visibility on the mailing list.
>>
>> You are right that a bitfield with at least one field generates a use of
>> `Bounded<$storage, ...>`, which requires the storage type to implement
>> `Integer`. However, `bitfield!` also accepts an empty field list. In that
>> case, no `Bounded` use is generated, so the `Integer` requirement is
>> absent, while the macro still generates the unconditional `Zeroable`
>> implementation.
>>
>> For example:
>>
>>     use core::num::NonZeroU32;
>>     use pin_init::Zeroable;
>>
>>     bitfield! {
>>         struct Bad(NonZeroU32) {}
>>     }
>>
>>     fn check_bad_zeroable() {
>>         let _: Bad = <Bad as Zeroable>::zeroed();
>>     }
>>
>> This currently compiles.
>
> No it doesn't.
>
> error[E0277]: the trait bound `core::num::NonZero<u32>: kernel::mem::AsRepr` is not satisfied
> 364 | /         bitfield! {
> 365 | |             struct Bad(NonZeroU32) {}
> 366 | |         }
>     | |_________^ the trait `kernel::mem::AsRepr` is not implemented for `core::num::NonZero<u32>`

On the other hand, it makes sense for `NonZero<u32>` to implement
`AsRepr<Repr = u32>`. So I think we do need better defense.

Given that fundamental `bitfield!` needs a plain integer type, I don't think we
want a `Zeroable` bound, but rather just enforce it has to be integer
primitives.

Given the recent `Integer` sealing, ad a check that $storage implements
`Integer` is probably the best check.

Best,
Gary


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

end of thread, other threads:[~2026-09-21 10:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-13 15:34 [PATCH] rust: bitfield: require Zeroable storage for Zeroable impl Yilin Chen
2026-09-13 19:37 ` Gary Guo
2026-09-21  6:32   ` Yilin Chen
2026-09-21  7:18     ` Alexandre Courbot
2026-09-21 10:05       ` Gary Guo

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®