Re: [PATCH 1/4] task_struct: Allow randomized layout

From: Kees Cook
Date: Mon Mar 26 2018 - 13:43:35 EST


On Mon, Mar 26, 2018 at 5:03 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> On Mon, Mar 26, 2018 at 01:52:46PM +0200, Peter Zijlstra wrote:
>
>> That now looks like:
>>
>> struct task_struct {
>> struct thread_info thread_info; /* 0 16 */
>> volatile long int state; /* 16 8 */
>>
>> /* XXX 40 bytes hole, try to pack */
>>
>> /* --- cacheline 1 boundary (64 bytes) --- */
>> struct {
>> void * stack; /* 64 8 */
>> atomic_t usage; /* 72 4 */
>> unsigned int flags; /* 76 4 */
>> unsigned int ptrace; /* 80 4 */
>> struct llist_node wake_entry; /* 88 8 */
>>
>>
>> Can we please undo this crap?
>
> The below gets rid of that nonsense.
>
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index e2c7f4369eff..767cf74d61f7 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -242,6 +242,15 @@
> #if defined(RANDSTRUCT_PLUGIN) && !defined(__CHECKER__)
> #define __randomize_layout __attribute__((randomize_layout))
> #define __no_randomize_layout __attribute__((no_randomize_layout))
> +/*
> + * RANDSTRUCT_PLUGIN wants to use an anonymous struct, but it is only
> + * possible since GCC 4.6. To provide as much build testing coverage
> + * as possible, this is used for all GCC 4.6+ builds, and not just on
> + * RANDSTRUCT_PLUGIN builds.
> + */
> +#define randomized_struct_fields_start struct {
> +#define randomized_struct_fields_end } __randomize_layout;
> +
> #endif
>
> #endif /* GCC_VERSION >= 40500 */
> @@ -256,15 +265,6 @@
> */
> #define __visible __attribute__((externally_visible))
>
> -/*
> - * RANDSTRUCT_PLUGIN wants to use an anonymous struct, but it is only
> - * possible since GCC 4.6. To provide as much build testing coverage
> - * as possible, this is used for all GCC 4.6+ builds, and not just on
> - * RANDSTRUCT_PLUGIN builds.
> - */
> -#define randomized_struct_fields_start struct {
> -#define randomized_struct_fields_end } __randomize_layout;
> -
> #endif /* GCC_VERSION >= 40600 */
>
>

This is fine by me, though obviously the comment would need to be updated. :)

-Kees

--
Kees Cook
Pixel Security