Re: [PATCH v2] rust: allow Clang-native `RANDSTRUCT` configs

["Gary Guo" <gary@garyguo.net>]

On Mon Mar 23, 2026 at 1:02 PM GMT, Miguel Ojeda wrote:
> The kernel supports `RANDSTRUCT_FULL` with Clang 16+, and `bindgen`
> (which uses `libclang` under the hood) inherits the information, so the
> generated bindings look correct.
>
> For instance, running:
>
>     bindgen x.h -- -frandomize-layout-seed=100
>
> with:
>
>     struct S1 {
>         int a;
>         int b;
>     };
>
>     struct S2 {
>         int a;
>         int b;
>     } __attribute__((randomize_layout));
>
>     struct S3 {
>         void (*a)(void);
>         void (*b)(void);
>     };
>
>     struct S4 {
>         void (*a)(void);
>         void (*b)(void);
>     } __attribute__((no_randomize_layout));
>
> may swap `S2`'s and `S3`'s members, but not `S1`'s nor `S4`'s:
>
>     pub struct S1 {
>         pub a: ::std::os::raw::c_int,
>         pub b: ::std::os::raw::c_int,
>     }
>
>     pub struct S2 {
>         pub b: ::std::os::raw::c_int,
>         pub a: ::std::os::raw::c_int,
>     }
>
>     pub struct S3 {
>         pub b: ::std::option::Option<unsafe extern "C" fn()>,
>         pub a: ::std::option::Option<unsafe extern "C" fn()>,
>     }
>
>     pub struct S4 {
>         pub a: ::std::option::Option<unsafe extern "C" fn()>,
>         pub b: ::std::option::Option<unsafe extern "C" fn()>,
>     }
>
> Thus allow those configurations by requiring a Clang compiler to use
> `RANDSTRUCT`. In addition, remove the `!GCC_PLUGIN_RANDSTRUCT` check
> since it is not needed.
>
> A simpler alternative would be to remove the `!RANDSTRUCT` check (keeping
> the `!GCC_PLUGIN_RANDSTRUCT` one). However, if in the future GCC starts
> supporting `RANDSTRUCT` natively, it would be likely that it would not
> work unless GCC and Clang agree on the exact semantics of the flag. And,
> as far as I can see, so far the randomization in Clang does not seem to be
> guaranteed to remain stable across versions or platforms, i.e. only for a
> given compiler Clang binary, given it is not documented and that LLVM's
> `HEAD` has the revert in place for the expected field names in the test
> (LLVM commit 8dbc6b560055 ("Revert "[randstruct] Check final randomized
> layout ordering"")) [1][2]. And the GCC plugin definitely does not match,
> e.g. its RNG is different (`std::mt19937` vs Bob Jenkins').
>
> And given it is not supposed to be guaranteed to remain stable across
> versions, it is a good idea to match the Clang and `bindgen`'s
> `libclang` versions -- we already have a warning for that in
> `scripts/rust_is_available.sh`. In the future, it would also be good to
> have a build test to double-check layouts do actually match (but that
> is true regardless of this particular feature).
>
> Finally, make a few required small changes to take into account the
> anonymous struct used in `randomized_struct_fields_*` in `struct
> task_struct`.

Can we avoid cfgs by always wrap things inside anonymous struct?

The comment says "This anon struct can add padding, so only enable it under
randstruct.", I wonder how much bigger the task_struct will be if this becomes
unconditional wrapping.

Best,
Gary

>
> [ Andreas writes:
>
>     Tested with the rust null block driver patch series [1]. I did a
>     few functional verification tests and a set of performance tests.
>
>     For the rnull driver, enabling randstruct with this patch gives no
>     statistically significant change to the average performance of the set
>     of 120 workloads that we publish on [2]. Individual configurations see
>     around 10% change in throughput, both directions.
>
>     Link: https://lore.kernel.org/rust-for-linux/20260216-rnull-v6-19-rc5-send-v1-0-de9a7af4b469@kernel.org/ [1]
>     Link: https://rust-for-linux.com/null-block-driver [2]
>
>         - Miguel ]
>
> Cc: Aaron Ballman <aaron@aaronballman.com>
> Cc: Bill Wendling <isanbard@gmail.com>
> Cc: Cole Nixon <nixontcole@gmail.com>
> Cc: Connor Kuehl <cipkuehl@gmail.com>
> Cc: Fangrui Song <i@maskray.me>
> Cc: James Foster <jafosterja@gmail.com>
> Cc: Jeff Takahashi <jeffrey.takahashi@gmail.com>
> Cc: Jordan Cantrell <jordan.cantrell@mail.com>
> Cc: Justin Stitt <justinstitt@google.com>
> Cc: Matthew Maurer <mmaurer@google.com>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Nicolas Schier <nsc@kernel.org>
> Cc: Nikk Forbus <nicholas.forbus@gmail.com>
> Cc: Qing Zhao <qing.zhao@oracle.com>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: Tim Pugh <nwtpugh@gmail.com>
> Link: https://reviews.llvm.org/D121556
> Link: https://github.com/llvm/llvm-project/commit/8dbc6b560055ff5068ff45b550482ba62c36b5a5 [1]
> Link: https://reviews.llvm.org/D124199 [2]
> Reviewed-by: Kees Cook <kees@kernel.org>
> Tested-by: Andreas Hindborg <a.hindborg@kernel.org>
> Link: https://patch.msgid.link/20241119185747.862544-1-ojeda@kernel.org
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> Ok, here is a rebased one in case it helps. Nowadays we have a couple
> more instances in Binder.
>
> I ran all the KUnit tests (nowadays we have way more compared to the
> days of v1) with a bunch of debug options and for a few architectures.
> Also did a run of all released Rust versions with x86_64. It would still
> be nice to get more actual testing like Andreas' did with his driver,
> but we will probably only get that if we actually go ahead...
>
> Kees: do you want to do the other change directly?
>
>  drivers/android/binder/deferred_close.rs | 29 ++++++++++++++++++---
>  init/Kconfig                             |  3 +--
>  rust/kernel/task.rs                      | 33 +++++++++++++++++++++---
>  3 files changed, 57 insertions(+), 8 deletions(-)
>