lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CA+tqQ4+Xp_Uv+O32JgCyN0vB-AJEaJdUWoWDOx0nTogeiDbj6w@mail.gmail.com>
Date: Mon, 29 Dec 2025 21:29:49 +0900
From: Jesung Yang <y.j3ms.n@...il.com>
To: Benno Lossin <lossin@...nel.org>
Cc: Miguel Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, 
	Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, 
	Alexandre Courbot <acourbot@...dia.com>, linux-kernel@...r.kernel.org, 
	rust-for-linux@...r.kernel.org, nouveau@...ts.freedesktop.org
Subject: Re: [PATCH v4 1/4] rust: macros: add derive macro for `Into`

On Sat, Dec 27, 2025 at 1:57 PM Benno Lossin <lossin@...nel.org> wrote:
> On Fri Dec 26, 2025 at 10:36 AM CET, Jesung Yang wrote:
> > On Fri, Dec 26, 2025 at 2:40 AM Benno Lossin <lossin@...nel.org> wrote:
> >> On Thu Dec 25, 2025 at 9:37 AM CET, Jesung Yang via B4 Relay wrote:

[...]

> >> > +    // Note on field-less `repr(C)` enums (quote from [1]):
> >> > +    //
> >> > +    //   In C, enums with discriminants that do not all fit into an `int` or all fit into an
> >> > +    //   `unsigned int` are a portability hazard: such enums are only permitted since C23, and not
> >> > +    //   supported e.g. by MSVC.
> >> > +    //
> >> > +    //   Furthermore, Rust interprets the discriminant values of `repr(C)` enums as expressions of
> >> > +    //   type `isize`. This makes it impossible to implement the C23 behavior of enums where the
> >> > +    //   enum discriminants have no predefined type and instead the enum uses a type large enough
> >> > +    //   to hold all discriminants.
> >> > +    //
> >> > +    //   Therefore, `repr(C)` enums in Rust require that either all discriminants to fit into a C
> >> > +    //   `int` or they all fit into an `unsigned int`.
> >> > +    //
> >> > +    // As such, `isize` is a reasonable representation for `repr(C)` enums, as it covers the range
> >> > +    //  of both `int` and `unsigned int`.
> >> > +    //
> >> > +    // For more information, see:
> >> > +    // - https://github.com/rust-lang/rust/issues/124403
> >> > +    // - https://github.com/rust-lang/rust/pull/147017
> >> > +    // - https://github.com/rust-lang/rust/blob/2ca7bcd03b87b52f7055a59b817443b0ac4a530d/compiler/rustc_lint_defs/src/builtin.rs#L5251-L5263 [1]
> >> > +
> >> > +    // Extract the representation passed by `#[repr(...)]` if present. If nothing is
> >> > +    // specified, the default is `Rust` representation, which uses `isize` for its
> >> > +    // discriminant type.
> >> > +    // See: https://doc.rust-lang.org/reference/items/enumerations.html#r-items.enum.discriminant.repr-rust
> >>
> >> I think we should error when no `#[repr({integer})]` attribute is
> >> specified.
> >
> > Not a blocker but just out of curiosity: are you concerned that the
> > default size might change in the future, leading to silent side
> > effects?
>
> `isize` already changes size when you switch between 64 and 32 bit
> architectures. I think the author of an enum they want to convert into
> integers should think about which size the enum should be.
>
> They already can choose `repr(isize)` if that is correct in that case.
> As a default, I would have choosen `i32` (but that conflicts with Rust's
> default, so we can't do it).

On second thought, I've realized that enforcing an error when
`#[repr({integer})]` is missing would prevent users from deriving
traits for `#[repr(C)]` enums. This is because rustc currently rejects
the combined `#[repr(C, {integer})]` syntax. For example, a user might
want to do this:

    #[derive(Into)]
    #[into(u8)]
    #[repr(C)]
    enum Foo {
        A,
        B,
    }

In this case, the code wouldn't compile if we strictly require
`#[repr({integer})]`, even if the user carefully picked `u8`, keeping
the enum's size (more precisely, the discriminant range) in mind.

Since we already perform a compile-time check to ensure all
discriminants fit within the types specified in the helper attributes,
I believe `#[repr({integer})]` isn't strictly required. Even with
`isize` changing size across architectures, our compile-time check
remains effective and will catch any overflows.

To sum up, I see three options:

    1. Drop support for `#[repr(C)]` enums entirely.
    2. Special-case `#[repr(C)]` enums: allow them to default to
       `isize`, otherwise require `#[repr({integer})]`.
    3. Permit missing `#[repr({integer})]` generally.

I am personally leaning toward Option 3, since our existing
compile-time check provides a sufficient safety margin to allow this
flexibility.

Thoughts on these?

Best regards,
Jesung

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ