* [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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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
2026-09-22 5:28 ` [PATCH v2] rust: bitfield: require integer storage Yilin Chen
2026-09-22 13:39 ` [PATCH] rust: bitfield: require Zeroable storage for Zeroable impl Alexandre Courbot
0 siblings, 2 replies; 9+ 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] 9+ messages in thread
* [PATCH v2] rust: bitfield: require integer storage
2026-09-21 10:05 ` Gary Guo
@ 2026-09-22 5:28 ` Yilin Chen
2026-09-22 13:53 ` Alexandre Courbot
2026-09-22 13:39 ` [PATCH] rust: bitfield: require Zeroable storage for Zeroable impl Alexandre Courbot
1 sibling, 1 reply; 9+ messages in thread
From: Yilin Chen @ 2026-09-22 5:28 UTC (permalink / raw)
To: gary, acourbot
Cc: rust-for-linux, linux-kernel, yury.norov, ojeda, Yilin Chen
The bitfield! macro generates an unconditional Zeroable implementation
for its wrapper type. An empty field list generates no Bounded usage, so
the storage type can bypass the Integer requirement.
Require the storage type to implement the sealed Integer trait for the
generated Zeroable implementation. This ensures that bitfield storage is
limited to primitive integer types with a valid all-zero bit pattern.
Fixes: b7b8b4ccdad4 ("rust: extract `bitfield!` macro from `register!`")
Assisted-by: GPT-5.6 Sol
Signed-off-by: Yilin Chen <1479826151@qq.com>
---
Changes in v2:
- Add `where $storage: ::kernel::num::Integer` bound.
- Update `// SAFETY` section.
---
I track the default rust-next branch, and there is not any code about
`AsRepr` in that branch. So in patch v1, I didn't know that case could
not compile. Thank you for your feedback!
rust/kernel/bitfield.rs | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
index a0d089423f21..b1fc98d7f8c3 100644
--- a/rust/kernel/bitfield.rs
+++ b/rust/kernel/bitfield.rs
@@ -330,8 +330,13 @@ impl $name {
}
}
- // SAFETY: `$storage` is `Zeroable` and `$name` is transparent.
- unsafe impl ::pin_init::Zeroable for $name {}
+ // SAFETY:
+ // - `$storage: Integer` is sealed to primitive integer types, for which the all-zero bit
+ // pattern is valid.
+ // - `$name` is `repr(transparent)` over `$storage`.
+ unsafe impl ::pin_init::Zeroable for $name
+ where $storage: ::kernel::num::Integer
+ {}
impl ::core::convert::From<$name> for $storage {
#[inline(always)]
--
2.25.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] rust: bitfield: require Zeroable storage for Zeroable impl
2026-09-21 10:05 ` Gary Guo
2026-09-22 5:28 ` [PATCH v2] rust: bitfield: require integer storage Yilin Chen
@ 2026-09-22 13:39 ` Alexandre Courbot
1 sibling, 0 replies; 9+ messages in thread
From: Alexandre Courbot @ 2026-09-22 13:39 UTC (permalink / raw)
To: Gary Guo; +Cc: Yilin Chen, rust-for-linux, linux-kernel, yury.norov, ojeda
On Mon Sep 21, 2026 at 7:05 PM JST, Gary Guo wrote:
> 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.
That, or we only derive `Zeroable` on a bitfield if its storage type
also implements `Zeroable`.
... but that possibly leaves other ways in which the invariant for
`NonZero` would not be enforced, and I don't see the point for a
bitfield to be backed by anything but a primitive type, so maybe
limiting storage to implementors of `Integer` is the right move indeed.
Yilin, do you want to send a patch for this?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] rust: bitfield: require integer storage
2026-09-22 5:28 ` [PATCH v2] rust: bitfield: require integer storage Yilin Chen
@ 2026-09-22 13:53 ` Alexandre Courbot
2026-09-22 14:02 ` Gary Guo
0 siblings, 1 reply; 9+ messages in thread
From: Alexandre Courbot @ 2026-09-22 13:53 UTC (permalink / raw)
To: Yilin Chen; +Cc: gary, rust-for-linux, linux-kernel, yury.norov, ojeda
On Tue Sep 22, 2026 at 2:28 PM JST, Yilin Chen wrote:
> The bitfield! macro generates an unconditional Zeroable implementation
> for its wrapper type. An empty field list generates no Bounded usage, so
> the storage type can bypass the Integer requirement.
>
> Require the storage type to implement the sealed Integer trait for the
> generated Zeroable implementation. This ensures that bitfield storage is
> limited to primitive integer types with a valid all-zero bit pattern.
>
> Fixes: b7b8b4ccdad4 ("rust: extract `bitfield!` macro from `register!`")
> Assisted-by: GPT-5.6 Sol
> Signed-off-by: Yilin Chen <1479826151@qq.com>
Looks like my wish [1] has been instantly granted.
[1] https://lore.kernel.org/rust-for-linux/DLLVY7G20JJA.2B0KTTODUU9FG@nvidia.com/
> ---
> Changes in v2:
> - Add `where $storage: ::kernel::num::Integer` bound.
> - Update `// SAFETY` section.
> ---
>
> I track the default rust-next branch, and there is not any code about
> `AsRepr` in that branch. So in patch v1, I didn't know that case could
> not compile. Thank you for your feedback!
>
> rust/kernel/bitfield.rs | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
> index a0d089423f21..b1fc98d7f8c3 100644
> --- a/rust/kernel/bitfield.rs
> +++ b/rust/kernel/bitfield.rs
> @@ -330,8 +330,13 @@ impl $name {
> }
> }
>
> - // SAFETY: `$storage` is `Zeroable` and `$name` is transparent.
> - unsafe impl ::pin_init::Zeroable for $name {}
> + // SAFETY:
> + // - `$storage: Integer` is sealed to primitive integer types, for which the all-zero bit
> + // pattern is valid.
> + // - `$name` is `repr(transparent)` over `$storage`.
> + unsafe impl ::pin_init::Zeroable for $name
> + where $storage: ::kernel::num::Integer
> + {}
While I guess that would somehow work, I think the proper place to do
this is the struct definition, i.e:
$vis struct $name
where
$storage: $crate::num::Integer,
{
inner: $storage,
}
This makes the error message also more explicit about what the problem
is (a bitfield requires an `Integer`, not merely the ability to
initialize it to zero).
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] rust: bitfield: require integer storage
2026-09-22 13:53 ` Alexandre Courbot
@ 2026-09-22 14:02 ` Gary Guo
0 siblings, 0 replies; 9+ messages in thread
From: Gary Guo @ 2026-09-22 14:02 UTC (permalink / raw)
To: Alexandre Courbot, Yilin Chen
Cc: gary, rust-for-linux, linux-kernel, yury.norov, ojeda
On Tue Sep 22, 2026 at 2:53 PM BST, Alexandre Courbot wrote:
>>
>> - // SAFETY: `$storage` is `Zeroable` and `$name` is transparent.
>> - unsafe impl ::pin_init::Zeroable for $name {}
>> + // SAFETY:
>> + // - `$storage: Integer` is sealed to primitive integer types, for which the all-zero bit
>> + // pattern is valid.
>> + // - `$name` is `repr(transparent)` over `$storage`.
>> + unsafe impl ::pin_init::Zeroable for $name
>> + where $storage: ::kernel::num::Integer
>> + {}
>
> While I guess that would somehow work, I think the proper place to do
> this is the struct definition, i.e:
>
> $vis struct $name
> where
> $storage: $crate::num::Integer,
> {
> inner: $storage,
> }
>
> This makes the error message also more explicit about what the problem
> is (a bitfield requires an `Integer`, not merely the ability to
> initialize it to zero).
Agreed.
Best,
Gary
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-09-22 14:02 UTC | newest]
Thread overview: 9+ 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
2026-09-22 5:28 ` [PATCH v2] rust: bitfield: require integer storage Yilin Chen
2026-09-22 13:53 ` Alexandre Courbot
2026-09-22 14:02 ` Gary Guo
2026-09-22 13:39 ` [PATCH] rust: bitfield: require Zeroable storage for Zeroable impl Alexandre Courbot
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®