From: Alice Ryhl <aliceryhl@google.com>
To: Matthew Maurer <mmaurer@google.com>
Cc: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Sami Tolvanen" <samitolvanen@google.com>,
"Kees Cook" <kees@kernel.org>,
"Nathan Chancellor" <nathan@kernel.org>,
"Carlos Llamas" <cmllamas@google.com>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Ramon de C Valle" <rcvalle@google.com>,
"Boqun Feng" <boqun.feng@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Trevor Gross" <tmgross@umich.edu>,
"Danilo Krummrich" <dakr@kernel.org>,
linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org
Subject: Re: [PATCH] rust: declare cfi_encoding for lru_status
Date: Tue, 6 Jan 2026 09:28:21 +0000 [thread overview]
Message-ID: <aVzVtS8c4MMEdcZe@google.com> (raw)
In-Reply-To: <CAGSQo00Vz54=3tgw8z3DtEqtXZGbk3SbJoautuD2pCgw4aswLA@mail.gmail.com>
On Mon, Jan 05, 2026 at 09:19:53AM -0800, Matthew Maurer wrote:
> On Mon, Jan 5, 2026 at 8:12 AM Alice Ryhl <aliceryhl@google.com> 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.
> >
> > To fix this, tell bindgen to generate a newtype and set the CFI type
> > explicitly. Note that we need to set the CFI attribute explicitly as
> > bindgen is using repr(transparent), which is otherwise identical to the
> > inner type for ABI purposes.
> >
> > This allows us to remove the page range helper C function in Binder
> > without risking a CFI failure when list_lru_walk calls the provided
> > function pointer.
> >
> > This requires bindgen v0.71 or greater.
> >
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> > drivers/android/binder/Makefile | 3 +--
> > drivers/android/binder/page_range.rs | 6 +++---
> > drivers/android/binder/page_range_helper.c | 24 ------------------------
> > drivers/android/binder/page_range_helper.h | 15 ---------------
> > rust/bindgen_parameters | 4 ++++
> > rust/bindings/bindings_helper.h | 1 -
> > rust/bindings/lib.rs | 1 +
> > rust/uapi/lib.rs | 1 +
> > 8 files changed, 10 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/android/binder/Makefile b/drivers/android/binder/Makefile
> > index 09eabb527fa092b659559367705fd3667db6cb2c..7e0cd9782a8b24db598034e15e5a36eca91b3fa9 100644
> > --- a/drivers/android/binder/Makefile
> > +++ b/drivers/android/binder/Makefile
> > @@ -5,5 +5,4 @@ obj-$(CONFIG_ANDROID_BINDER_IPC_RUST) += rust_binder.o
> > rust_binder-y := \
> > rust_binder_main.o \
> > rust_binderfs.o \
> > - rust_binder_events.o \
> > - page_range_helper.o
> > + rust_binder_events.o
> > diff --git a/drivers/android/binder/page_range.rs b/drivers/android/binder/page_range.rs
> > index 9379038f61f513c51ebed6c7e7b6fde32e5b8d06..eb738e169525839a199132dd71e69e0b9cc69053 100644
> > --- a/drivers/android/binder/page_range.rs
> > +++ b/drivers/android/binder/page_range.rs
> > @@ -642,15 +642,15 @@ fn drop(self: Pin<&mut Self>) {
> > unsafe {
> > bindings::list_lru_walk(
> > list_lru,
> > - Some(bindings::rust_shrink_free_page_wrap),
> > + Some(rust_shrink_free_page),
> > ptr::null_mut(),
> > nr_to_scan,
> > )
> > }
> > }
> >
> > -const LRU_SKIP: bindings::lru_status = bindings::lru_status_LRU_SKIP;
> > -const LRU_REMOVED_ENTRY: bindings::lru_status = bindings::lru_status_LRU_REMOVED_RETRY;
> > +const LRU_SKIP: bindings::lru_status = bindings::lru_status::LRU_SKIP;
> > +const LRU_REMOVED_ENTRY: bindings::lru_status = bindings::lru_status::LRU_REMOVED_RETRY;
> >
> > /// # Safety
> > /// Called by the shrinker.
> > diff --git a/drivers/android/binder/page_range_helper.c b/drivers/android/binder/page_range_helper.c
> > deleted file mode 100644
> > index 496887723ee003e910d6ce67dbadd8c5286e39d1..0000000000000000000000000000000000000000
> > --- a/drivers/android/binder/page_range_helper.c
> > +++ /dev/null
> > @@ -1,24 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -
> > -/* C helper for page_range.rs to work around a CFI violation.
> > - *
> > - * Bindgen currently pretends that `enum lru_status` is the same as an integer.
> > - * This assumption is fine ABI-wise, but once you add CFI to the mix, it
> > - * triggers a CFI violation because `enum lru_status` gets a different CFI tag.
> > - *
> > - * This file contains a workaround until bindgen can be fixed.
> > - *
> > - * Copyright (C) 2025 Google LLC.
> > - */
> > -#include "page_range_helper.h"
> > -
> > -unsigned int rust_shrink_free_page(struct list_head *item,
> > - struct list_lru_one *list,
> > - void *cb_arg);
> > -
> > -enum lru_status
> > -rust_shrink_free_page_wrap(struct list_head *item, struct list_lru_one *list,
> > - void *cb_arg)
> > -{
> > - return rust_shrink_free_page(item, list, cb_arg);
> > -}
> > diff --git a/drivers/android/binder/page_range_helper.h b/drivers/android/binder/page_range_helper.h
> > deleted file mode 100644
> > index 18dd2dd117b253fcbac735b48032b8f2d53d11fe..0000000000000000000000000000000000000000
> > --- a/drivers/android/binder/page_range_helper.h
> > +++ /dev/null
> > @@ -1,15 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0 */
> > -/*
> > - * Copyright (C) 2025 Google, Inc.
> > - */
> > -
> > -#ifndef _LINUX_PAGE_RANGE_HELPER_H
> > -#define _LINUX_PAGE_RANGE_HELPER_H
> > -
> > -#include <linux/list_lru.h>
> > -
> > -enum lru_status
> > -rust_shrink_free_page_wrap(struct list_head *item, struct list_lru_one *list,
> > - void *cb_arg);
> > -
> > -#endif /* _LINUX_PAGE_RANGE_HELPER_H */
> > diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
> > index fd2fd1c3cb9a51ea46fcd721907783b457aa1378..1358f3348ffdd31f9bef6c04ee9577d0f6a0c5a6 100644
> > --- a/rust/bindgen_parameters
> > +++ b/rust/bindgen_parameters
> > @@ -23,6 +23,10 @@
> > # warning. We don't need to peek into it anyway.
> > --opaque-type spinlock
> >
> > +# enums that appear in indirect function calls should specify a cfi type
> > +--newtype-enum lru_status
> > +--with-attribute-custom-enum=lru_status='#[cfi_encoding="lru_status"]'
>
> I think this may need to be `#[cfi_encoding="10lru_status"]`
Ah, I did have some trouble testing this.
Is there an easy way to test that I got the cfi enconding right? I guess
I can always add a kunit test that makes a dynamic function call...
Alice
next prev parent reply other threads:[~2026-01-06 9:28 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-05 16:12 [PATCH] rust: declare cfi_encoding for lru_status Alice Ryhl
2026-01-05 17:19 ` Matthew Maurer
2026-01-06 9:28 ` Alice Ryhl [this message]
2026-01-06 20:01 ` Kees Cook
2026-01-06 20:18 ` Matthew Maurer
2026-01-06 20:25 ` Kees Cook
2026-01-06 20:31 ` Matthew Maurer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aVzVtS8c4MMEdcZe@google.com \
--to=aliceryhl@google.com \
--cc=a.hindborg@kernel.org \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=cmllamas@google.com \
--cc=dakr@kernel.org \
--cc=gary@garyguo.net \
--cc=gregkh@linuxfoundation.org \
--cc=kees@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=mmaurer@google.com \
--cc=nathan@kernel.org \
--cc=ojeda@kernel.org \
--cc=rcvalle@google.com \
--cc=rust-for-linux@vger.kernel.org \
--cc=samitolvanen@google.com \
--cc=tmgross@umich.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.