[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGSQo00Vz54=3tgw8z3DtEqtXZGbk3SbJoautuD2pCgw4aswLA@mail.gmail.com>
Date: Mon, 5 Jan 2026 09:19:53 -0800
From: Matthew Maurer <mmaurer@...gle.com>
To: Alice Ryhl <aliceryhl@...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 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
I think this may need to be `#[cfi_encoding="10lru_status"]`
> +--with-attribute-custom-enum=lru_status='#[cfi_encoding="lru_status"]'
> +
> # `seccomp`'s comment gets understood as a doctest
> --no-doc-comments
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index a067038b4b422b4256f4a2b75fe644d47e6e82c8..cc12cf1614eac38bcbc1c634e6e7e6d8ccfec434 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -147,5 +147,4 @@ const vm_flags_t RUST_CONST_HELPER_VM_NOHUGEPAGE = VM_NOHUGEPAGE;
> #if IS_ENABLED(CONFIG_ANDROID_BINDER_IPC_RUST)
> #include "../../drivers/android/binder/rust_binder.h"
> #include "../../drivers/android/binder/rust_binder_events.h"
> -#include "../../drivers/android/binder/page_range_helper.h"
> #endif
> diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
> index 0c57cf9b4004f176997c59ecc58a9a9ac76163d9..7f72ab66eebe6ef4227ce1b210d66a7867cbf5dd 100644
> --- a/rust/bindings/lib.rs
> +++ b/rust/bindings/lib.rs
> @@ -23,6 +23,7 @@
> unreachable_pub,
> unsafe_op_in_unsafe_fn
> )]
> +#![feature(cfi_encoding)]
>
> #[allow(dead_code)]
> #[allow(clippy::cast_lossless)]
> diff --git a/rust/uapi/lib.rs b/rust/uapi/lib.rs
> index 1d5fd9efb93e9db97fec84fca2bae37b500c20c5..83c4795acbff1da852639bcbd9bcf5fb66b7e070 100644
> --- a/rust/uapi/lib.rs
> +++ b/rust/uapi/lib.rs
> @@ -28,6 +28,7 @@
> unsafe_op_in_unsafe_fn
> )]
> #![cfg_attr(CONFIG_RUSTC_HAS_UNNECESSARY_TRANSMUTES, allow(unnecessary_transmutes))]
> +#![feature(cfi_encoding)]
>
> // Manual definition of blocklisted types.
> type __kernel_size_t = usize;
>
> ---
> base-commit: 9ace4753a5202b02191d54e9fdf7f9e3d02b85eb
> change-id: 20260105-cfi-lru-status-60d05fe6f93b
>
> Best regards,
> --
> Alice Ryhl <aliceryhl@...gle.com>
>
Powered by blists - more mailing lists