[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aVzVtS8c4MMEdcZe@google.com>
Date: Tue, 6 Jan 2026 09:28:21 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Matthew Maurer <mmaurer@...gle.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Sami Tolvanen <samitolvanen@...gle.com>,
Kees Cook <kees@...nel.org>, 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 Mon, Jan 05, 2026 at 09:19:53AM -0800, Matthew Maurer wrote:
> On Mon, Jan 5, 2026 at 8:12 AM Alice Ryhl <aliceryhl@...gle.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@...gle.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
Powered by blists - more mailing lists