[PATCH] rust: init: remove impl Zeroable for Infallible

[PATCH] rust: init: remove impl Zeroable for Infallible

[Benno Lossin]

From: Laine Taffin Altman <alexanderaltman@me.com>

It is not enough for a type to be a ZST to guarantee that zeroed memory
is a valid value for it; it must also be inhabited. Creating a value of
an uninhabited type, ZST or no, is immediate UB.
Thus remove the implementation of `Zeroable` for `Infallible`, since
that type is not inhabited.

Cc: stable@vger.kernel.org
Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
Signed-off-by: Laine Taffin Altman <alexanderaltman@me.com>
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
 rust/kernel/init.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 424257284d16..538e03cfc84a 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -1292,8 +1292,8 @@ macro_rules! impl_zeroable {
     i8, i16, i32, i64, i128, isize,
     f32, f64,
 
-    // SAFETY: These are ZSTs, there is nothing to zero.
-    {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, Infallible, (),
+    // SAFETY: These are inhabited ZSTs, there is nothing to zero and a valid value exists.
+    {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, (),
 
     // SAFETY: Type is allowed to take any value, including all zeros.
     {<T>} MaybeUninit<T>,

base-commit: 768409cff6cc89fe1194da880537a09857b6e4db
-- 
2.42.0

Re: [PATCH] rust: init: remove impl Zeroable for Infallible

[Alice Ryhl]

On Thu, Mar 14, 2024 at 12:09 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> From: Laine Taffin Altman <alexanderaltman@me.com>
>
> It is not enough for a type to be a ZST to guarantee that zeroed memory
> is a valid value for it; it must also be inhabited. Creating a value of
> an uninhabited type, ZST or no, is immediate UB.
> Thus remove the implementation of `Zeroable` for `Infallible`, since
> that type is not inhabited.
>
> Cc: stable@vger.kernel.org
> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
> Signed-off-by: Laine Taffin Altman <alexanderaltman@me.com>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>

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

Re: [PATCH] rust: init: remove impl Zeroable for Infallible

[Boqun Feng]

On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
> From: Laine Taffin Altman <alexanderaltman@me.com>
> 
> It is not enough for a type to be a ZST to guarantee that zeroed memory
> is a valid value for it; it must also be inhabited. Creating a value of
> an uninhabited type, ZST or no, is immediate UB.
> Thus remove the implementation of `Zeroable` for `Infallible`, since
> that type is not inhabited.
> 
> Cc: stable@vger.kernel.org
> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
> Signed-off-by: Laine Taffin Altman <alexanderaltman@me.com>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>

I think either in the commit log or in the code comment, there better be
a link or explanation on "(un)inhabited type". The rest looks good to
me.

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>

Regards,
Boqun

> ---
>  rust/kernel/init.rs | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index 424257284d16..538e03cfc84a 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -1292,8 +1292,8 @@ macro_rules! impl_zeroable {
>      i8, i16, i32, i64, i128, isize,
>      f32, f64,
>  
> -    // SAFETY: These are ZSTs, there is nothing to zero.
> -    {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, Infallible, (),
> +    // SAFETY: These are inhabited ZSTs, there is nothing to zero and a valid value exists.
> +    {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, (),
>  
>      // SAFETY: Type is allowed to take any value, including all zeros.
>      {<T>} MaybeUninit<T>,
> 
> base-commit: 768409cff6cc89fe1194da880537a09857b6e4db
> -- 
> 2.42.0
> 
> 
> 

Re: [PATCH] rust: init: remove impl Zeroable for Infallible

[Laine Taffin Altman]

On Mar 18, 2024, at 10:25 AM, Boqun Feng <boqun.feng@gmail.com> wrote:
> On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
>> From: Laine Taffin Altman <alexanderaltman@me.com>
>> 
>> It is not enough for a type to be a ZST to guarantee that zeroed memory
>> is a valid value for it; it must also be inhabited. Creating a value of
>> an uninhabited type, ZST or no, is immediate UB.
>> Thus remove the implementation of `Zeroable` for `Infallible`, since
>> that type is not inhabited.
>> 
>> Cc: stable@vger.kernel.org
>> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
>> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
>> Signed-off-by: Laine Taffin Altman <alexanderaltman@me.com>
>> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> 
> I think either in the commit log or in the code comment, there better be
> a link or explanation on "(un)inhabited type". The rest looks good to
> me.

Would the following be okay for that purpose?

A type is inhabited if at least one valid value of that type exists; a
type is uninhabited if no valid values of that type exist.  The terms
"inhabited" and "uninhabited" in this sense originate in type theory,
a branch of mathematics.

In Rust, producing an invalid value of any type is immediate undefined
behavior (UB); this includes via zeroing memory.  Therefore, since an
uninhabited type has no valid values, producing any values at all for
it is UB.

The Rust standard library type `core::convert::Infallible` is
uninhabited, by virtue of having been declared as an enum with no
cases, which always produces uninhabited types in Rust.  Thus, remove
the implementation of `Zeroable` for `Infallible`, thereby avoiding
the UB.

Thanks,
Laine

> 
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> 
> Regards,
> Boqun
> 
>> ---
>> rust/kernel/init.rs | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
>> index 424257284d16..538e03cfc84a 100644
>> --- a/rust/kernel/init.rs
>> +++ b/rust/kernel/init.rs
>> @@ -1292,8 +1292,8 @@ macro_rules! impl_zeroable {
>>     i8, i16, i32, i64, i128, isize,
>>     f32, f64,
>> 
>> -    // SAFETY: These are ZSTs, there is nothing to zero.
>> -    {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, Infallible, (),
>> +    // SAFETY: These are inhabited ZSTs, there is nothing to zero and a valid value exists.
>> +    {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, (),
>> 
>>     // SAFETY: Type is allowed to take any value, including all zeros.
>>     {<T>} MaybeUninit<T>,
>> 
>> base-commit: 768409cff6cc89fe1194da880537a09857b6e4db
>> -- 
>> 2.42.0
>> 
>> 
>> 

Re: [PATCH] rust: init: remove impl Zeroable for Infallible

[Boqun Feng]

On Mon, Mar 18, 2024 at 08:17:07PM -0700, Laine Taffin Altman wrote:
> On Mar 18, 2024, at 10:25 AM, Boqun Feng <boqun.feng@gmail.com> wrote:
> > On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
> >> From: Laine Taffin Altman <alexanderaltman@me.com>
> >> 
> >> It is not enough for a type to be a ZST to guarantee that zeroed memory
> >> is a valid value for it; it must also be inhabited. Creating a value of
> >> an uninhabited type, ZST or no, is immediate UB.
> >> Thus remove the implementation of `Zeroable` for `Infallible`, since
> >> that type is not inhabited.
> >> 
> >> Cc: stable@vger.kernel.org
> >> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
> >> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
> >> Signed-off-by: Laine Taffin Altman <alexanderaltman@me.com>
> >> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> > 
> > I think either in the commit log or in the code comment, there better be
> > a link or explanation on "(un)inhabited type". The rest looks good to
> > me.
> 
> Would the following be okay for that purpose?
> 
> A type is inhabited if at least one valid value of that type exists; a
> type is uninhabited if no valid values of that type exist.  The terms
> "inhabited" and "uninhabited" in this sense originate in type theory,
> a branch of mathematics.
> 
> In Rust, producing an invalid value of any type is immediate undefined
> behavior (UB); this includes via zeroing memory.  Therefore, since an
> uninhabited type has no valid values, producing any values at all for
> it is UB.
> 
> The Rust standard library type `core::convert::Infallible` is
> uninhabited, by virtue of having been declared as an enum with no
> cases, which always produces uninhabited types in Rust.  Thus, remove
> the implementation of `Zeroable` for `Infallible`, thereby avoiding
> the UB.
> 

Yeah, this works for me. Thanks!

Regards,
Boqun

> Thanks,
> Laine
> 
> > 
> > Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> > 
> > Regards,
> > Boqun
> > 
> >> ---
> >> rust/kernel/init.rs | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> >> index 424257284d16..538e03cfc84a 100644
> >> --- a/rust/kernel/init.rs
> >> +++ b/rust/kernel/init.rs
> >> @@ -1292,8 +1292,8 @@ macro_rules! impl_zeroable {
> >>     i8, i16, i32, i64, i128, isize,
> >>     f32, f64,
> >> 
> >> -    // SAFETY: These are ZSTs, there is nothing to zero.
> >> -    {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, Infallible, (),
> >> +    // SAFETY: These are inhabited ZSTs, there is nothing to zero and a valid value exists.
> >> +    {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, (),
> >> 
> >>     // SAFETY: Type is allowed to take any value, including all zeros.
> >>     {<T>} MaybeUninit<T>,
> >> 
> >> base-commit: 768409cff6cc89fe1194da880537a09857b6e4db
> >> -- 
> >> 2.42.0
> >> 
> >> 
> >> 
> 

Re: [PATCH] rust: init: remove impl Zeroable for Infallible

[Laine Taffin Altman]

On Mar 18, 2024, at 9:39 PM, Boqun Feng <boqun.feng@gmail.com> wrote:
> On Mon, Mar 18, 2024 at 08:17:07PM -0700, Laine Taffin Altman wrote:
>> On Mar 18, 2024, at 10:25 AM, Boqun Feng <boqun.feng@gmail.com> wrote:
>>> On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
>>>> From: Laine Taffin Altman <alexanderaltman@me.com>
>>>> 
>>>> It is not enough for a type to be a ZST to guarantee that zeroed memory
>>>> is a valid value for it; it must also be inhabited. Creating a value of
>>>> an uninhabited type, ZST or no, is immediate UB.
>>>> Thus remove the implementation of `Zeroable` for `Infallible`, since
>>>> that type is not inhabited.
>>>> 
>>>> Cc: stable@vger.kernel.org
>>>> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
>>>> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
>>>> Signed-off-by: Laine Taffin Altman <alexanderaltman@me.com>
>>>> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
>>> 
>>> I think either in the commit log or in the code comment, there better be
>>> a link or explanation on "(un)inhabited type". The rest looks good to
>>> me.
>> 
>> Would the following be okay for that purpose?
>> 
>> A type is inhabited if at least one valid value of that type exists; a
>> type is uninhabited if no valid values of that type exist.  The terms
>> "inhabited" and "uninhabited" in this sense originate in type theory,
>> a branch of mathematics.
>> 
>> In Rust, producing an invalid value of any type is immediate undefined
>> behavior (UB); this includes via zeroing memory.  Therefore, since an
>> uninhabited type has no valid values, producing any values at all for
>> it is UB.
>> 
>> The Rust standard library type `core::convert::Infallible` is
>> uninhabited, by virtue of having been declared as an enum with no
>> cases, which always produces uninhabited types in Rust.  Thus, remove
>> the implementation of `Zeroable` for `Infallible`, thereby avoiding
>> the UB.
>> 
> 
> Yeah, this works for me. Thanks!

Great!  Should it be re-sent or can the new wording be incorporated upon merge?

Thank,
Laine

> 
> Regards,
> Boqun
> 
>> Thanks,
>> Laine
>> 
>>> 
>>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>>> 
>>> Regards,
>>> Boqun
>>> 
>>>> ---
>>>> rust/kernel/init.rs | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
>>>> index 424257284d16..538e03cfc84a 100644
>>>> --- a/rust/kernel/init.rs
>>>> +++ b/rust/kernel/init.rs
>>>> @@ -1292,8 +1292,8 @@ macro_rules! impl_zeroable {
>>>>    i8, i16, i32, i64, i128, isize,
>>>>    f32, f64,
>>>> 
>>>> -    // SAFETY: These are ZSTs, there is nothing to zero.
>>>> -    {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, Infallible, (),
>>>> +    // SAFETY: These are inhabited ZSTs, there is nothing to zero and a valid value exists.
>>>> +    {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, (),
>>>> 
>>>>    // SAFETY: Type is allowed to take any value, including all zeros.
>>>>    {<T>} MaybeUninit<T>,
>>>> 
>>>> base-commit: 768409cff6cc89fe1194da880537a09857b6e4db
>>>> -- 
>>>> 2.42.0
>>>> 
>>>> 
>>>> 
>> 

Re: [PATCH] rust: init: remove impl Zeroable for Infallible

[Benno Lossin]

On 3/19/24 06:28, Laine Taffin Altman wrote:
> On Mar 18, 2024, at 9:39 PM, Boqun Feng <boqun.feng@gmail.com> wrote:
>> On Mon, Mar 18, 2024 at 08:17:07PM -0700, Laine Taffin Altman wrote:
>>> On Mar 18, 2024, at 10:25 AM, Boqun Feng <boqun.feng@gmail.com> wrote:
>>>> On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
>>>>> From: Laine Taffin Altman <alexanderaltman@me.com>
>>>>>
>>>>> It is not enough for a type to be a ZST to guarantee that zeroed memory
>>>>> is a valid value for it; it must also be inhabited. Creating a value of
>>>>> an uninhabited type, ZST or no, is immediate UB.
>>>>> Thus remove the implementation of `Zeroable` for `Infallible`, since
>>>>> that type is not inhabited.
>>>>>
>>>>> Cc: stable@vger.kernel.org
>>>>> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
>>>>> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
>>>>> Signed-off-by: Laine Taffin Altman <alexanderaltman@me.com>
>>>>> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
>>>>
>>>> I think either in the commit log or in the code comment, there better be
>>>> a link or explanation on "(un)inhabited type". The rest looks good to
>>>> me.
>>>
>>> Would the following be okay for that purpose?
>>>
>>> A type is inhabited if at least one valid value of that type exists; a
>>> type is uninhabited if no valid values of that type exist.  The terms
>>> "inhabited" and "uninhabited" in this sense originate in type theory,
>>> a branch of mathematics.
>>>
>>> In Rust, producing an invalid value of any type is immediate undefined
>>> behavior (UB); this includes via zeroing memory.  Therefore, since an
>>> uninhabited type has no valid values, producing any values at all for
>>> it is UB.
>>>
>>> The Rust standard library type `core::convert::Infallible` is
>>> uninhabited, by virtue of having been declared as an enum with no
>>> cases, which always produces uninhabited types in Rust.  Thus, remove
>>> the implementation of `Zeroable` for `Infallible`, thereby avoiding
>>> the UB.
>>>
>>
>> Yeah, this works for me. Thanks!
> 
> Great!  Should it be re-sent or can the new wording be incorporated upon merge?

I can re-send it for you again, or do you want to send it yourself?
I think it is also a good idea to add a link to [1] in the code, since
the above explanation is rather long and fits better in the commit
message.

-- 
Cheers,
Benno

[1]: https://doc.rust-lang.org/nomicon/exotic-sizes.html#empty-types

Re: [PATCH] rust: init: remove impl Zeroable for Infallible

[Miguel Ojeda]

On Tue, Mar 19, 2024 at 11:34 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> I can re-send it for you again, or do you want to send it yourself?
> I think it is also a good idea to add a link to [1] in the code, since
> the above explanation is rather long and fits better in the commit
> message.

Agreed, if you want to have a note in the code itself (to avoid
mistakes re-adding them, I imagine), then I would say a short sentence
+ link is best.

Your link is a good one for an explanation, since it mentions
explicitly the UB. The reference's list [1] would be a good fit for
non-explanation purposes -- it mentions explicitly `!` (and
`Infallible` is supposed to eventually be an alias as far as I know).

In addition, while it is not important in this case (i.e. most likely
nobody is affected), it doesn't hurt to include an example that shows
the issue in general for this sort of patches, i.e. what kind of code
will be prevented from compiling, e.g.

    pr_info!("{}",
Box::<core::convert::Infallible>::init(kernel::init::zeroed())?);

In any case, even v1 looks good to me -- thanks!

[1] https://doc.rust-lang.org/reference/behavior-considered-undefined.html

Cheers,
Miguel

Re: [PATCH] rust: init: remove impl Zeroable for Infallible

[Laine Taffin Altman]

On Mar 19, 2024, at 3:34 AM, Benno Lossin <benno.lossin@proton.me> wrote:
> On 3/19/24 06:28, Laine Taffin Altman wrote:
>> On Mar 18, 2024, at 9:39 PM, Boqun Feng <boqun.feng@gmail.com> wrote:
>>> On Mon, Mar 18, 2024 at 08:17:07PM -0700, Laine Taffin Altman wrote:
>>>> On Mar 18, 2024, at 10:25 AM, Boqun Feng <boqun.feng@gmail.com> wrote:
>>>>> On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
>>>>>> From: Laine Taffin Altman <alexanderaltman@me.com>
>>>>>> 
>>>>>> It is not enough for a type to be a ZST to guarantee that zeroed memory
>>>>>> is a valid value for it; it must also be inhabited. Creating a value of
>>>>>> an uninhabited type, ZST or no, is immediate UB.
>>>>>> Thus remove the implementation of `Zeroable` for `Infallible`, since
>>>>>> that type is not inhabited.
>>>>>> 
>>>>>> Cc: stable@vger.kernel.org
>>>>>> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
>>>>>> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
>>>>>> Signed-off-by: Laine Taffin Altman <alexanderaltman@me.com>
>>>>>> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
>>>>> 
>>>>> I think either in the commit log or in the code comment, there better be
>>>>> a link or explanation on "(un)inhabited type". The rest looks good to
>>>>> me.
>>>> 
>>>> Would the following be okay for that purpose?
>>>> 
>>>> A type is inhabited if at least one valid value of that type exists; a
>>>> type is uninhabited if no valid values of that type exist.  The terms
>>>> "inhabited" and "uninhabited" in this sense originate in type theory,
>>>> a branch of mathematics.
>>>> 
>>>> In Rust, producing an invalid value of any type is immediate undefined
>>>> behavior (UB); this includes via zeroing memory.  Therefore, since an
>>>> uninhabited type has no valid values, producing any values at all for
>>>> it is UB.
>>>> 
>>>> The Rust standard library type `core::convert::Infallible` is
>>>> uninhabited, by virtue of having been declared as an enum with no
>>>> cases, which always produces uninhabited types in Rust.  Thus, remove
>>>> the implementation of `Zeroable` for `Infallible`, thereby avoiding
>>>> the UB.
>>>> 
>>> 
>>> Yeah, this works for me. Thanks!
>> 
>> Great!  Should it be re-sent or can the new wording be incorporated upon merge?
> 
> I can re-send it for you again, or do you want to send it yourself?
> I think it is also a good idea to add a link to [1] in the code, since
> the above explanation is rather long and fits better in the commit
> message.
> 

I’ll try and do it myself; thank you for sending the first round for me and illustrating procedures!  What Reviewed-By’s/Signed-Off-By's should I retain?

Thanks,
Laine

> -- 
> Cheers,
> Benno
> 
> [1]: https://doc.rust-lang.org/nomicon/exotic-sizes.html#empty-types

Re: [PATCH] rust: init: remove impl Zeroable for Infallible

[Miguel Ojeda]

On Thu, Mar 21, 2024 at 5:53 AM Laine Taffin Altman
<alexanderaltman@me.com> wrote:
>
> I’ll try and do it myself; thank you for sending the first round for me and illustrating procedures!  What Reviewed-By’s/Signed-Off-By's should I retain?

For the Signed-off-by, only yours is OK. For the Reviewed-by, it
depends on how much you have changed, i.e. whether you consider their
review does not apply anymore -- please see
https://docs.kernel.org/process/submitting-patches.html#reviewer-s-statement-of-oversight

Cheers,
Miguel

Re: [PATCH] rust: init: remove impl Zeroable for Infallible

[Benno Lossin]

On 21.03.24 05:53, Laine Taffin Altman wrote:
> On Mar 19, 2024, at 3:34 AM, Benno Lossin <benno.lossin@proton.me> wrote:
>> On 3/19/24 06:28, Laine Taffin Altman wrote:
>>> On Mar 18, 2024, at 9:39 PM, Boqun Feng <boqun.feng@gmail.com> wrote:
>>>> On Mon, Mar 18, 2024 at 08:17:07PM -0700, Laine Taffin Altman wrote:
>>>>> On Mar 18, 2024, at 10:25 AM, Boqun Feng <boqun.feng@gmail.com> wrote:
>>>>>> On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
>>>>>>> From: Laine Taffin Altman <alexanderaltman@me.com>
>>>>>>>
>>>>>>> It is not enough for a type to be a ZST to guarantee that zeroed memory
>>>>>>> is a valid value for it; it must also be inhabited. Creating a value of
>>>>>>> an uninhabited type, ZST or no, is immediate UB.
>>>>>>> Thus remove the implementation of `Zeroable` for `Infallible`, since
>>>>>>> that type is not inhabited.
>>>>>>>
>>>>>>> Cc: stable@vger.kernel.org
>>>>>>> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
>>>>>>> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
>>>>>>> Signed-off-by: Laine Taffin Altman <alexanderaltman@me.com>
>>>>>>> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
>>>>>>
>>>>>> I think either in the commit log or in the code comment, there better be
>>>>>> a link or explanation on "(un)inhabited type". The rest looks good to
>>>>>> me.
>>>>>
>>>>> Would the following be okay for that purpose?
>>>>>
>>>>> A type is inhabited if at least one valid value of that type exists; a
>>>>> type is uninhabited if no valid values of that type exist.  The terms
>>>>> "inhabited" and "uninhabited" in this sense originate in type theory,
>>>>> a branch of mathematics.
>>>>>
>>>>> In Rust, producing an invalid value of any type is immediate undefined
>>>>> behavior (UB); this includes via zeroing memory.  Therefore, since an
>>>>> uninhabited type has no valid values, producing any values at all for
>>>>> it is UB.
>>>>>
>>>>> The Rust standard library type `core::convert::Infallible` is
>>>>> uninhabited, by virtue of having been declared as an enum with no
>>>>> cases, which always produces uninhabited types in Rust.  Thus, remove
>>>>> the implementation of `Zeroable` for `Infallible`, thereby avoiding
>>>>> the UB.
>>>>>
>>>>
>>>> Yeah, this works for me. Thanks!
>>>
>>> Great!  Should it be re-sent or can the new wording be incorporated upon merge?
>>
>> I can re-send it for you again, or do you want to send it yourself?
>> I think it is also a good idea to add a link to [1] in the code, since
>> the above explanation is rather long and fits better in the commit
>> message.
>>
> 
> I’ll try and do it myself; thank you for sending the first round for me and illustrating procedures!  What Reviewed-By’s/Signed-Off-By's should I retain?

Do you still want to send it yourself? If you don't have the time, no
problem, I can send it again.

-- 
Cheers,
Benno

Re: [PATCH] rust: init: remove impl Zeroable for Infallible

[Laine Taffin Altman]

I’ll do it myself tomorrow night; sorry for the delay!

Thanks,
Laine

On Mar 30, 2024, at 5:03 AM, Benno Lossin <benno.lossin@proton.me> wrote:
> 
> 
> <mime-attachment>
> <encrypted.asc>

Re: [PATCH] rust: init: remove impl Zeroable for Infallible

[Benno Lossin]

On 30.03.24 17:36, Laine Taffin Altman wrote:
> I’ll do it myself tomorrow night; sorry for the delay!

Great, and no worries (it's fine if you still need a couple days)!

> 
> Thanks,
> Laine
> 
> On Mar 30, 2024, at 5:03 AM, Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> 
>> <mime-attachment>
>> <encrypted.asc>

Sorry about that, I hope this one is not encrypted.

-- 
Cheers,
Benno