[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <202601061223.E39B079CA@keescook>
Date: Tue, 6 Jan 2026 12:25:34 -0800
From: Kees Cook <kees@...nel.org>
To: Matthew Maurer <mmaurer@...gle.com>
Cc: Alice Ryhl <aliceryhl@...gle.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Sami Tolvanen <samitolvanen@...gle.com>,
Nathan Chancellor <nathan@...nel.org>,
Carlos Llamas <cmllamas@...gle.com>,
Miguel Ojeda <ojeda@...nel.org>,
Ramon de C Valle <rcvalle@...gle.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <lossin@...nel.org>,
Andreas Hindborg <a.hindborg@...nel.org>,
Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>, linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org
Subject: Re: [PATCH] rust: declare cfi_encoding for lru_status
On Tue, Jan 06, 2026 at 12:18:41PM -0800, Matthew Maurer wrote:
> On Tue, Jan 6, 2026 at 12:01 PM Kees Cook <kees@...nel.org> wrote:
> >
> > On Mon, Jan 05, 2026 at 04:12:47PM +0000, Alice Ryhl wrote:
> > > By default bindgen will convert 'enum lru_status' into a typedef for an
> > > integer, but this leads to the wrong cfi type. It's supposed to be a
> > > type called "lru_status" rather than the underlying native integer type.
> >
> > Is this a bug in bindgen?
>
> Kind of, kind of not. The things that are interacting here:
>
> Rust assumes that enums have all their variants defined in its
> codegen. This means that if the Rust definition for an integer enum
> has variants A=0, B=1, and C=2, and we get sent `0xffff` from C,
> undefined behavior occurs. To mitigate that, `bindgen` usually (unless
> explicitly told otherwise) *does not* represent C enums as Rust enums.
> There are several ways to do this. The default, usually used in the
> kernel, is to represent them as integers of the correct size with a
> set of constants available to compare them against. Alice has added an
> exception for this type which switches it to use
> `#[repr(transparent)]` struct lru_status(int_of_appropriate_width)` as
> the type.
>
> The Rust CFI implementation assumes:
>
> 1. `#[repr(C)] struct MyStruct {` should have an encoding equivalent
> to a C type named `MyStruct` in the root namespace, similar to using
> `extern "C" {` in C++.
> 2. `#[repr(transparent)] struct Foo {` uses an encoding equivalent to
> that of its only non-zero-sized field. This is done because the usual
> reason for `#[repr(transparent)]` is to guarantee that the layout is
> identical to that field so you can safely transmute between the struct
> and its field in certain scenarios. There are several places in
> commonly used Rust libraries where CFI will report if
> `#[repr(transparent)] struct Foo { bar: Bar }` is not encoded
> equivalently to `Bar` as a result.
>
> If `bindgen` output `#[repr(C)] struct
> lru_status(int_of_appropriate_width)`, it would just work, since that
> encodes as a C struct named `lru_status`, which happens to be the same
> as a C enum named `lru_status`. However, outside the scope of CFI,
> labeling this as `#[repr(C)]` rather than `#[repr(transparent)]` makes
> no sense.
>
> What Alice is doing would create a struct with `#[cfi_encoding =
> "myencoding"] #[repr(transparent)] struct lru_status(i32);`, which
> would get treated as a transparent struct, but have the provided
> encoding.
>
> The possible fixes in bindgen would be to provide
> yet-another-enum-generation-mode which causes `#[repr(C)] struct
> lru_status(` to be generated (easiest) or to automatically attach
> `#[cfi_encoding = "correctencoding"]` for all declared types, possibly
> by querying clang first. This latter approach could theoretically come
> with an error to emit if CFI encodings were being generated but a type
> that would need one is using a raw integer (which can't have a CFI
> encoding attached).
I see; thanks for the detailed explanation! My main reason for asking
about this was wondering if this was going to be something that needed
to be fixed for all enums, or if this was more of a one-off. It sounds
like it's basically a one-off due to how the enum was chosen to be
mapped in this case, so that's less concerning. I guess we just need to
make note of this when choosing that kind of mapping in the future?
--
Kees Cook
Powered by blists - more mailing lists