[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAGSQo02jXz72=-9TUcbSUxOc0Yx9-3AA24mw1PjrwFP-ajq0+Q@mail.gmail.com>
Date: Tue, 6 Jan 2026 12:31:39 -0800
From: Matthew Maurer <mmaurer@...gle.com>
To: Kees Cook <kees@...nel.org>
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 6, 2026 at 12:25 PM Kees Cook <kees@...nel.org> wrote:
>
> 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 needs to be fixed in some form for any enum which appears in a
function pointer type where Rust either calls the function pointer or
defines a function that will be used as a function pointer. It would
be best if we got a comprehensive solution at some point (either all
`#[repr(C)]` or always attaching `#[cfi_encoding]`). It just isn't
common at the moment in C APIs to combine function pointers and `enum
foo`.
> 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?
Arguably the C code triggering this is doing the right thing by
referring to the type as `enum foo` rather than `int`. None of the
default Rust representations in `bindgen` will get us a compatible
representation, which results in things like people writing a manual
`#[cfi_encoding]`, so in an ideal world I would like to see something
done in bindgen about this.
>
> --
> Kees Cook
Powered by blists - more mailing lists