* [PATCH] rust: init: remove impl Zeroable for Infallible
@ 2024-03-13 23:09 Benno Lossin
2024-03-14 9:14 ` Alice Ryhl
2024-03-18 17:25 ` Boqun Feng
0 siblings, 2 replies; 13+ messages in thread
From: Benno Lossin @ 2024-03-13 23:09 UTC (permalink / raw
To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Martin Rodriguez Reboredo
Cc: Laine Taffin Altman, stable, rust-for-linux, linux-kernel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] rust: init: remove impl Zeroable for Infallible
2024-03-13 23:09 [PATCH] rust: init: remove impl Zeroable for Infallible Benno Lossin
@ 2024-03-14 9:14 ` Alice Ryhl
2024-03-18 17:25 ` Boqun Feng
1 sibling, 0 replies; 13+ messages in thread
From: Alice Ryhl @ 2024-03-14 9:14 UTC (permalink / raw
To: Benno Lossin, Laine Taffin Altman
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg,
Martin Rodriguez Reboredo, stable, rust-for-linux, linux-kernel
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>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rust: init: remove impl Zeroable for Infallible
2024-03-13 23:09 [PATCH] rust: init: remove impl Zeroable for Infallible Benno Lossin
2024-03-14 9:14 ` Alice Ryhl
@ 2024-03-18 17:25 ` Boqun Feng
2024-03-19 3:17 ` Laine Taffin Altman
1 sibling, 1 reply; 13+ messages in thread
From: Boqun Feng @ 2024-03-18 17:25 UTC (permalink / raw
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Martin Rodriguez Reboredo, Laine Taffin Altman, stable,
rust-for-linux, linux-kernel
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
>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rust: init: remove impl Zeroable for Infallible
2024-03-18 17:25 ` Boqun Feng
@ 2024-03-19 3:17 ` Laine Taffin Altman
2024-03-19 4:39 ` Boqun Feng
0 siblings, 1 reply; 13+ messages in thread
From: Laine Taffin Altman @ 2024-03-19 3:17 UTC (permalink / raw
To: Boqun Feng
Cc: Benno Lossin, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Martin Rodriguez Reboredo, stable, rust-for-linux, lkml
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
>>
>>
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rust: init: remove impl Zeroable for Infallible
2024-03-19 3:17 ` Laine Taffin Altman
@ 2024-03-19 4:39 ` Boqun Feng
2024-03-19 5:28 ` Laine Taffin Altman
0 siblings, 1 reply; 13+ messages in thread
From: Boqun Feng @ 2024-03-19 4:39 UTC (permalink / raw
To: Laine Taffin Altman
Cc: Benno Lossin, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Martin Rodriguez Reboredo, stable, rust-for-linux, lkml
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
> >>
> >>
> >>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rust: init: remove impl Zeroable for Infallible
2024-03-19 4:39 ` Boqun Feng
@ 2024-03-19 5:28 ` Laine Taffin Altman
2024-03-19 10:34 ` Benno Lossin
0 siblings, 1 reply; 13+ messages in thread
From: Laine Taffin Altman @ 2024-03-19 5:28 UTC (permalink / raw
To: Boqun Feng
Cc: Benno Lossin, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Martin Rodriguez Reboredo, stable, rust-for-linux, lkml
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
>>>>
>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rust: init: remove impl Zeroable for Infallible
2024-03-19 5:28 ` Laine Taffin Altman
@ 2024-03-19 10:34 ` Benno Lossin
2024-03-19 11:24 ` Miguel Ojeda
2024-03-21 4:53 ` Laine Taffin Altman
0 siblings, 2 replies; 13+ messages in thread
From: Benno Lossin @ 2024-03-19 10:34 UTC (permalink / raw
To: Laine Taffin Altman, Boqun Feng
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Martin Rodriguez Reboredo, stable, rust-for-linux, lkml
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rust: init: remove impl Zeroable for Infallible
2024-03-19 10:34 ` Benno Lossin
@ 2024-03-19 11:24 ` Miguel Ojeda
2024-03-21 4:53 ` Laine Taffin Altman
1 sibling, 0 replies; 13+ messages in thread
From: Miguel Ojeda @ 2024-03-19 11:24 UTC (permalink / raw
To: Benno Lossin
Cc: Laine Taffin Altman, Boqun Feng, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Martin Rodriguez Reboredo, stable,
rust-for-linux, lkml
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rust: init: remove impl Zeroable for Infallible
2024-03-19 10:34 ` Benno Lossin
2024-03-19 11:24 ` Miguel Ojeda
@ 2024-03-21 4:53 ` Laine Taffin Altman
2024-03-21 9:07 ` Miguel Ojeda
2024-03-30 12:03 ` Benno Lossin
1 sibling, 2 replies; 13+ messages in thread
From: Laine Taffin Altman @ 2024-03-21 4:53 UTC (permalink / raw
To: Benno Lossin
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Martin Rodriguez Reboredo, stable, rust-for-linux, lkml
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rust: init: remove impl Zeroable for Infallible
2024-03-21 4:53 ` Laine Taffin Altman
@ 2024-03-21 9:07 ` Miguel Ojeda
2024-03-30 12:03 ` Benno Lossin
1 sibling, 0 replies; 13+ messages in thread
From: Miguel Ojeda @ 2024-03-21 9:07 UTC (permalink / raw
To: Laine Taffin Altman
Cc: Benno Lossin, Boqun Feng, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Martin Rodriguez Reboredo, stable,
rust-for-linux, lkml
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rust: init: remove impl Zeroable for Infallible
2024-03-21 4:53 ` Laine Taffin Altman
2024-03-21 9:07 ` Miguel Ojeda
@ 2024-03-30 12:03 ` Benno Lossin
2024-03-30 16:36 ` Laine Taffin Altman
1 sibling, 1 reply; 13+ messages in thread
From: Benno Lossin @ 2024-03-30 12:03 UTC (permalink / raw
To: Laine Taffin Altman
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Martin Rodriguez Reboredo, stable, rust-for-linux, lkml
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rust: init: remove impl Zeroable for Infallible
2024-03-30 12:03 ` Benno Lossin
@ 2024-03-30 16:36 ` Laine Taffin Altman
2024-03-30 16:43 ` Benno Lossin
0 siblings, 1 reply; 13+ messages in thread
From: Laine Taffin Altman @ 2024-03-30 16:36 UTC (permalink / raw
To: Benno Lossin
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Martin Rodriguez Reboredo, stable, rust-for-linux, lkml
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>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] rust: init: remove impl Zeroable for Infallible
2024-03-30 16:36 ` Laine Taffin Altman
@ 2024-03-30 16:43 ` Benno Lossin
0 siblings, 0 replies; 13+ messages in thread
From: Benno Lossin @ 2024-03-30 16:43 UTC (permalink / raw
To: Laine Taffin Altman
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Martin Rodriguez Reboredo, stable, rust-for-linux, lkml
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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-03-30 16:43 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-13 23:09 [PATCH] rust: init: remove impl Zeroable for Infallible Benno Lossin
2024-03-14 9:14 ` Alice Ryhl
2024-03-18 17:25 ` Boqun Feng
2024-03-19 3:17 ` Laine Taffin Altman
2024-03-19 4:39 ` Boqun Feng
2024-03-19 5:28 ` Laine Taffin Altman
2024-03-19 10:34 ` Benno Lossin
2024-03-19 11:24 ` Miguel Ojeda
2024-03-21 4:53 ` Laine Taffin Altman
2024-03-21 9:07 ` Miguel Ojeda
2024-03-30 12:03 ` Benno Lossin
2024-03-30 16:36 ` Laine Taffin Altman
2024-03-30 16:43 ` Benno Lossin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.