[kernel-hardening] [PATCH v3 0/4] randstruct: Enable function pointer struct detection

[kernel-hardening] [PATCH v3 0/4] randstruct: Enable function pointer struct detection

[Kees Cook]

This continues the randstruct series (which is in -next now), and adds
explicit task_struct randomization and enables automatic structure
randomization.

The randstruct plugin (in -next), the marking of task_struct, and the
opt-out markings are modified from Brad Spengler/PaX Team's code in the
last public patch of grsecurity/PaX based on my understanding of the
code. Changes or omissions from the original code are mine and don't
reflect the original grsecurity/PaX code.

-Kees

[PATCH v3 0/4] randstruct: Enable function pointer struct detection

[Kees Cook]

This continues the randstruct series (which is in -next now), and adds
explicit task_struct randomization and enables automatic structure
randomization.

The randstruct plugin (in -next), the marking of task_struct, and the
opt-out markings are modified from Brad Spengler/PaX Team's code in the
last public patch of grsecurity/PaX based on my understanding of the
code. Changes or omissions from the original code are mine and don't
reflect the original grsecurity/PaX code.

-Kees

[kernel-hardening] [PATCH 1/4] task_struct: Allow randomized layout

[Kees Cook]

This marks most of the layout of task_struct as randomizable, but leaves
thread_info and scheduler state untouched at the start, and thread_struct
untouched at the end.

Other parts of the kernel use unnamed structures, but the 0-day builder
using gcc-4.4 blows up on static initializers. Officially, it's documented
as only working on gcc 4.6 and later, which further confuses me:
	https://gcc.gnu.org/wiki/C11Status
The structure layout randomization already requires gcc 4.7, but instead
of depending on the plugin being enabled, just check the gcc versions
for wider build testing. At Linus's suggestion, the marking is hidden
in a macro to reduce how ugly it looks. Additionally, indenting is left
unchanged since it would make things harder to read.

Randomization of task_struct is modified from Brad Spengler/PaX Team's
code in the last public patch of grsecurity/PaX based on my understanding
of the code. Changes or omissions from the original code are mine and
don't reflect the original grsecurity/PaX code.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/compiler-gcc.h | 13 ++++++++++++-
 include/linux/compiler.h     |  5 +++++
 include/linux/sched.h        | 14 ++++++++++++++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 7deaae3dc87d..c4a66c036692 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -231,6 +231,7 @@
 #endif /* GCC_VERSION >= 40500 */
 
 #if GCC_VERSION >= 40600
+
 /*
  * When used with Link Time Optimization, gcc can optimize away C functions or
  * variables which are referenced only from assembly code.  __visible tells the
@@ -238,7 +239,17 @@
  * this.
  */
 #define __visible	__attribute__((externally_visible))
-#endif
+
+/*
+ * 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 */
 
 
 #if GCC_VERSION >= 40900 && !defined(__CHECKER__)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 55ee9ee814f8..0b4ac3e8c63e 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -456,6 +456,11 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 # define __no_randomize_layout
 #endif
 
+#ifndef randomized_struct_fields_start
+# define randomized_struct_fields_start
+# define randomized_struct_fields_end
+#endif
+
 /*
  * Tell gcc if a function is cold. The compiler will assume any path
  * directly leading to the call is unlikely.
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f833254fce00..e2ad3531e7fe 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -490,6 +490,13 @@ struct task_struct {
 #endif
 	/* -1 unrunnable, 0 runnable, >0 stopped: */
 	volatile long			state;
+
+	/*
+	 * This begins the randomizable portion of task_struct. Only
+	 * scheduling-critical items should be added above here.
+	 */
+	randomized_struct_fields_start
+
 	void				*stack;
 	atomic_t			usage;
 	/* Per task flags (PF_*), defined further below: */
@@ -1051,6 +1058,13 @@ struct task_struct {
 	/* Used by LSM modules for access restriction: */
 	void				*security;
 #endif
+
+	/*
+	 * New fields for task_struct should be added above here, so that
+	 * they are included in the randomized portion of task_struct.
+	 */
+	randomized_struct_fields_end
+
 	/* CPU-specific state of this task: */
 	struct thread_struct		thread;
 
-- 
2.7.4

[PATCH 1/4] task_struct: Allow randomized layout

[Kees Cook]

This marks most of the layout of task_struct as randomizable, but leaves
thread_info and scheduler state untouched at the start, and thread_struct
untouched at the end.

Other parts of the kernel use unnamed structures, but the 0-day builder
using gcc-4.4 blows up on static initializers. Officially, it's documented
as only working on gcc 4.6 and later, which further confuses me:
	https://gcc.gnu.org/wiki/C11Status
The structure layout randomization already requires gcc 4.7, but instead
of depending on the plugin being enabled, just check the gcc versions
for wider build testing. At Linus's suggestion, the marking is hidden
in a macro to reduce how ugly it looks. Additionally, indenting is left
unchanged since it would make things harder to read.

Randomization of task_struct is modified from Brad Spengler/PaX Team's
code in the last public patch of grsecurity/PaX based on my understanding
of the code. Changes or omissions from the original code are mine and
don't reflect the original grsecurity/PaX code.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/compiler-gcc.h | 13 ++++++++++++-
 include/linux/compiler.h     |  5 +++++
 include/linux/sched.h        | 14 ++++++++++++++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 7deaae3dc87d..c4a66c036692 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -231,6 +231,7 @@
 #endif /* GCC_VERSION >= 40500 */
 
 #if GCC_VERSION >= 40600
+
 /*
  * When used with Link Time Optimization, gcc can optimize away C functions or
  * variables which are referenced only from assembly code.  __visible tells the
@@ -238,7 +239,17 @@
  * this.
  */
 #define __visible	__attribute__((externally_visible))
-#endif
+
+/*
+ * 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 */
 
 
 #if GCC_VERSION >= 40900 && !defined(__CHECKER__)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 55ee9ee814f8..0b4ac3e8c63e 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -456,6 +456,11 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 # define __no_randomize_layout
 #endif
 
+#ifndef randomized_struct_fields_start
+# define randomized_struct_fields_start
+# define randomized_struct_fields_end
+#endif
+
 /*
  * Tell gcc if a function is cold. The compiler will assume any path
  * directly leading to the call is unlikely.
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f833254fce00..e2ad3531e7fe 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -490,6 +490,13 @@ struct task_struct {
 #endif
 	/* -1 unrunnable, 0 runnable, >0 stopped: */
 	volatile long			state;
+
+	/*
+	 * This begins the randomizable portion of task_struct. Only
+	 * scheduling-critical items should be added above here.
+	 */
+	randomized_struct_fields_start
+
 	void				*stack;
 	atomic_t			usage;
 	/* Per task flags (PF_*), defined further below: */
@@ -1051,6 +1058,13 @@ struct task_struct {
 	/* Used by LSM modules for access restriction: */
 	void				*security;
 #endif
+
+	/*
+	 * New fields for task_struct should be added above here, so that
+	 * they are included in the randomized portion of task_struct.
+	 */
+	randomized_struct_fields_end
+
 	/* CPU-specific state of this task: */
 	struct thread_struct		thread;
 
-- 
2.7.4

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

[Peter Zijlstra]

On Mon, Jun 19, 2017 at 01:56:38PM -0700, Kees Cook wrote:
> This marks most of the layout of task_struct as randomizable, but leaves
> thread_info and scheduler state untouched at the start, and thread_struct
> untouched at the end.
> 
> Other parts of the kernel use unnamed structures, but the 0-day builder
> using gcc-4.4 blows up on static initializers. Officially, it's documented
> as only working on gcc 4.6 and later, which further confuses me:
> 	https://gcc.gnu.org/wiki/C11Status
> The structure layout randomization already requires gcc 4.7, but instead
> of depending on the plugin being enabled, just check the gcc versions
> for wider build testing. At Linus's suggestion, the marking is hidden
> in a macro to reduce how ugly it looks. Additionally, indenting is left
> unchanged since it would make things harder to read.
> 
> Randomization of task_struct is modified from Brad Spengler/PaX Team's
> code in the last public patch of grsecurity/PaX based on my understanding
> of the code. Changes or omissions from the original code are mine and
> don't reflect the original grsecurity/PaX code.
> 
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>

Thanks for the Cc :/

> +#define randomized_struct_fields_start	struct {
> +#define randomized_struct_fields_end	} __randomize_layout;

> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index f833254fce00..e2ad3531e7fe 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -490,6 +490,13 @@ struct task_struct {
>  #endif
>  	/* -1 unrunnable, 0 runnable, >0 stopped: */
>  	volatile long			state;
> +
> +	/*
> +	 * This begins the randomizable portion of task_struct. Only
> +	 * scheduling-critical items should be added above here.
> +	 */
> +	randomized_struct_fields_start
> +
>  	void				*stack;
>  	atomic_t			usage;
>  	/* Per task flags (PF_*), defined further below: */


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?

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

[Peter Zijlstra]

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 */
 
 

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

[Kees Cook]

On Mon, Mar 26, 2018 at 5:03 AM, Peter Zijlstra <peterz@infradead.org> 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

[kernel-hardening] [PATCH 2/4] randstruct: opt-out externally exposed function pointer structs

[Kees Cook]

Some function pointer structures are used externally to the kernel, like
the paravirt structures. These should never be randomized, so mark them as
such.

These markings are verbatim from Brad Spengler/PaX Team's code in the last
public patch of grsecurity/PaX based on my understanding of the code. Changes
or omissions from the original code are mine and don't reflect the original
grsecurity/PaX code.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/arm/include/asm/cacheflush.h     |  2 +-
 arch/x86/include/asm/paravirt_types.h | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
index d69bebf697e7..74504b154256 100644
--- a/arch/arm/include/asm/cacheflush.h
+++ b/arch/arm/include/asm/cacheflush.h
@@ -116,7 +116,7 @@ struct cpu_cache_fns {
 	void (*dma_unmap_area)(const void *, size_t, int);
 
 	void (*dma_flush_range)(const void *, const void *);
-};
+} __no_randomize_layout;
 
 /*
  * Select the calling method
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 7465d6fe336f..96c7e3cf43fa 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -83,7 +83,7 @@ struct pv_init_ops {
 	 */
 	unsigned (*patch)(u8 type, u16 clobber, void *insnbuf,
 			  unsigned long addr, unsigned len);
-};
+} __no_randomize_layout;
 
 
 struct pv_lazy_ops {
@@ -91,12 +91,12 @@ struct pv_lazy_ops {
 	void (*enter)(void);
 	void (*leave)(void);
 	void (*flush)(void);
-};
+} __no_randomize_layout;
 
 struct pv_time_ops {
 	unsigned long long (*sched_clock)(void);
 	unsigned long long (*steal_clock)(int cpu);
-};
+} __no_randomize_layout;
 
 struct pv_cpu_ops {
 	/* hooks for various privileged instructions */
@@ -175,7 +175,7 @@ struct pv_cpu_ops {
 
 	void (*start_context_switch)(struct task_struct *prev);
 	void (*end_context_switch)(struct task_struct *next);
-};
+} __no_randomize_layout;
 
 struct pv_irq_ops {
 	/*
@@ -198,7 +198,7 @@ struct pv_irq_ops {
 #ifdef CONFIG_X86_64
 	void (*adjust_exception_frame)(void);
 #endif
-};
+} __no_randomize_layout;
 
 struct pv_mmu_ops {
 	unsigned long (*read_cr2)(void);
@@ -306,7 +306,7 @@ struct pv_mmu_ops {
 	   an mfn.  We can tell which is which from the index. */
 	void (*set_fixmap)(unsigned /* enum fixed_addresses */ idx,
 			   phys_addr_t phys, pgprot_t flags);
-};
+} __no_randomize_layout;
 
 struct arch_spinlock;
 #ifdef CONFIG_SMP
@@ -323,7 +323,7 @@ struct pv_lock_ops {
 	void (*kick)(int cpu);
 
 	struct paravirt_callee_save vcpu_is_preempted;
-};
+} __no_randomize_layout;
 
 /* This contains all the paravirt structures: we get a convenient
  * number for each function using the offset which we use to indicate
@@ -335,7 +335,7 @@ struct paravirt_patch_template {
 	struct pv_irq_ops pv_irq_ops;
 	struct pv_mmu_ops pv_mmu_ops;
 	struct pv_lock_ops pv_lock_ops;
-};
+} __no_randomize_layout;
 
 extern struct pv_info pv_info;
 extern struct pv_init_ops pv_init_ops;
-- 
2.7.4

[PATCH 2/4] randstruct: opt-out externally exposed function pointer structs

[Kees Cook]

Some function pointer structures are used externally to the kernel, like
the paravirt structures. These should never be randomized, so mark them as
such.

These markings are verbatim from Brad Spengler/PaX Team's code in the last
public patch of grsecurity/PaX based on my understanding of the code. Changes
or omissions from the original code are mine and don't reflect the original
grsecurity/PaX code.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/arm/include/asm/cacheflush.h     |  2 +-
 arch/x86/include/asm/paravirt_types.h | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
index d69bebf697e7..74504b154256 100644
--- a/arch/arm/include/asm/cacheflush.h
+++ b/arch/arm/include/asm/cacheflush.h
@@ -116,7 +116,7 @@ struct cpu_cache_fns {
 	void (*dma_unmap_area)(const void *, size_t, int);
 
 	void (*dma_flush_range)(const void *, const void *);
-};
+} __no_randomize_layout;
 
 /*
  * Select the calling method
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 7465d6fe336f..96c7e3cf43fa 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -83,7 +83,7 @@ struct pv_init_ops {
 	 */
 	unsigned (*patch)(u8 type, u16 clobber, void *insnbuf,
 			  unsigned long addr, unsigned len);
-};
+} __no_randomize_layout;
 
 
 struct pv_lazy_ops {
@@ -91,12 +91,12 @@ struct pv_lazy_ops {
 	void (*enter)(void);
 	void (*leave)(void);
 	void (*flush)(void);
-};
+} __no_randomize_layout;
 
 struct pv_time_ops {
 	unsigned long long (*sched_clock)(void);
 	unsigned long long (*steal_clock)(int cpu);
-};
+} __no_randomize_layout;
 
 struct pv_cpu_ops {
 	/* hooks for various privileged instructions */
@@ -175,7 +175,7 @@ struct pv_cpu_ops {
 
 	void (*start_context_switch)(struct task_struct *prev);
 	void (*end_context_switch)(struct task_struct *next);
-};
+} __no_randomize_layout;
 
 struct pv_irq_ops {
 	/*
@@ -198,7 +198,7 @@ struct pv_irq_ops {
 #ifdef CONFIG_X86_64
 	void (*adjust_exception_frame)(void);
 #endif
-};
+} __no_randomize_layout;
 
 struct pv_mmu_ops {
 	unsigned long (*read_cr2)(void);
@@ -306,7 +306,7 @@ struct pv_mmu_ops {
 	   an mfn.  We can tell which is which from the index. */
 	void (*set_fixmap)(unsigned /* enum fixed_addresses */ idx,
 			   phys_addr_t phys, pgprot_t flags);
-};
+} __no_randomize_layout;
 
 struct arch_spinlock;
 #ifdef CONFIG_SMP
@@ -323,7 +323,7 @@ struct pv_lock_ops {
 	void (*kick)(int cpu);
 
 	struct paravirt_callee_save vcpu_is_preempted;
-};
+} __no_randomize_layout;
 
 /* This contains all the paravirt structures: we get a convenient
  * number for each function using the offset which we use to indicate
@@ -335,7 +335,7 @@ struct paravirt_patch_template {
 	struct pv_irq_ops pv_irq_ops;
 	struct pv_mmu_ops pv_mmu_ops;
 	struct pv_lock_ops pv_lock_ops;
-};
+} __no_randomize_layout;
 
 extern struct pv_info pv_info;
 extern struct pv_init_ops pv_init_ops;
-- 
2.7.4

[kernel-hardening] [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Kees Cook]

Since the ACPICA source is maintained externally to the kernel, we can
neither switch it to designated initializers nor mark it
__no_randomize_layout. Until ACPICA-upstream changes[1] land to handle the
designated initialization, explicitly skip it in the plugin.

[1] https://github.com/acpica/acpica/pull/248

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 scripts/gcc-plugins/randomize_layout_plugin.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
index f777ead58ba8..e6e02a40d522 100644
--- a/scripts/gcc-plugins/randomize_layout_plugin.c
+++ b/scripts/gcc-plugins/randomize_layout_plugin.c
@@ -346,6 +346,10 @@ static int relayout_struct(tree type)
 	    !strcmp((const char *)ORIG_TYPE_NAME(type), "RAWPCIFACTORY"))
 		return 0;
 
+	/* Skip ACPICA structs until refreshed with designated_init. */
+	if (!strcmp((const char *)ORIG_TYPE_NAME(type), "acpi_sleep_functions"))
+		return 0;
+
 	/* throw out any structs in uapi */
 	xloc = expand_location(DECL_SOURCE_LOCATION(TYPE_FIELDS(type)));
 
-- 
2.7.4

[PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Kees Cook]

Since the ACPICA source is maintained externally to the kernel, we can
neither switch it to designated initializers nor mark it
__no_randomize_layout. Until ACPICA-upstream changes[1] land to handle the
designated initialization, explicitly skip it in the plugin.

[1] https://github.com/acpica/acpica/pull/248

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 scripts/gcc-plugins/randomize_layout_plugin.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
index f777ead58ba8..e6e02a40d522 100644
--- a/scripts/gcc-plugins/randomize_layout_plugin.c
+++ b/scripts/gcc-plugins/randomize_layout_plugin.c
@@ -346,6 +346,10 @@ static int relayout_struct(tree type)
 	    !strcmp((const char *)ORIG_TYPE_NAME(type), "RAWPCIFACTORY"))
 		return 0;
 
+	/* Skip ACPICA structs until refreshed with designated_init. */
+	if (!strcmp((const char *)ORIG_TYPE_NAME(type), "acpi_sleep_functions"))
+		return 0;
+
 	/* throw out any structs in uapi */
 	xloc = expand_location(DECL_SOURCE_LOCATION(TYPE_FIELDS(type)));
 
-- 
2.7.4

[kernel-hardening] Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Christoph Hellwig]

On Mon, Jun 19, 2017 at 01:56:40PM -0700, Kees Cook wrote:
> Since the ACPICA source is maintained externally to the kernel, we can
> neither switch it to designated initializers nor mark it
> __no_randomize_layout. Until ACPICA-upstream changes[1] land to handle the
> designated initialization, explicitly skip it in the plugin.

NAK.  ACPI has no business rejecting kernel changes to start with, but
independent of that your patch was complete garbage anyway.

There is no need for function pointers here, please include the patch
below instead:

---
>From e8046f6507c2ed60bc501a0c0caa5a3f15f5e3e4 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Sun, 28 May 2017 09:53:45 +0300
Subject: acpi: get rid of acpi_sleep_dispatch

No need for the array of structs of function pointers when we can just
call the handfull of functions directly.

This could be further cleaned up if acpi_gbl_reduced_hardware was defined
true in the ACPI_REDUCED_HARDWARE case, but that's material for the next
round.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/acpi/acpica/hwxfsleep.c | 89 +++++++++--------------------------------
 include/acpi/actypes.h          |  9 -----
 2 files changed, 18 insertions(+), 80 deletions(-)

diff --git a/drivers/acpi/acpica/hwxfsleep.c b/drivers/acpi/acpica/hwxfsleep.c
index 5733b1167e46..66fa3ebddd57 100644
--- a/drivers/acpi/acpica/hwxfsleep.c
+++ b/drivers/acpi/acpica/hwxfsleep.c
@@ -57,26 +57,6 @@ acpi_hw_set_firmware_waking_vector(struct acpi_table_facs *facs,
 				   acpi_physical_address physical_address64);
 #endif
 
-static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id);
-
-/*
- * Dispatch table used to efficiently branch to the various sleep
- * functions.
- */
-#define ACPI_SLEEP_FUNCTION_ID         0
-#define ACPI_WAKE_PREP_FUNCTION_ID     1
-#define ACPI_WAKE_FUNCTION_ID          2
-
-/* Legacy functions are optional, based upon ACPI_REDUCED_HARDWARE */
-
-static struct acpi_sleep_functions acpi_sleep_dispatch[] = {
-	{ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_sleep),
-	 acpi_hw_extended_sleep},
-	{ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake_prep),
-	 acpi_hw_extended_wake_prep},
-	{ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake), acpi_hw_extended_wake}
-};
-
 /*
  * These functions are removed for the ACPI_REDUCED_HARDWARE case:
  *      acpi_set_firmware_waking_vector
@@ -236,53 +216,6 @@ acpi_status acpi_enter_sleep_state_s4bios(void)
 
 ACPI_EXPORT_SYMBOL(acpi_enter_sleep_state_s4bios)
 #endif				/* !ACPI_REDUCED_HARDWARE */
-/*******************************************************************************
- *
- * FUNCTION:    acpi_hw_sleep_dispatch
- *
- * PARAMETERS:  sleep_state         - Which sleep state to enter/exit
- *              function_id         - Sleep, wake_prep, or Wake
- *
- * RETURN:      Status from the invoked sleep handling function.
- *
- * DESCRIPTION: Dispatch a sleep/wake request to the appropriate handling
- *              function.
- *
- ******************************************************************************/
-static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id)
-{
-	acpi_status status;
-	struct acpi_sleep_functions *sleep_functions =
-	    &acpi_sleep_dispatch[function_id];
-
-#if (!ACPI_REDUCED_HARDWARE)
-	/*
-	 * If the Hardware Reduced flag is set (from the FADT), we must
-	 * use the extended sleep registers (FADT). Note: As per the ACPI
-	 * specification, these extended registers are to be used for HW-reduced
-	 * platforms only. They are not general-purpose replacements for the
-	 * legacy PM register sleep support.
-	 */
-	if (acpi_gbl_reduced_hardware) {
-		status = sleep_functions->extended_function(sleep_state);
-	} else {
-		/* Legacy sleep */
-
-		status = sleep_functions->legacy_function(sleep_state);
-	}
-
-	return (status);
-
-#else
-	/*
-	 * For the case where reduced-hardware-only code is being generated,
-	 * we know that only the extended sleep registers are available
-	 */
-	status = sleep_functions->extended_function(sleep_state);
-	return (status);
-
-#endif				/* !ACPI_REDUCED_HARDWARE */
-}
 
 /*******************************************************************************
  *
@@ -389,7 +322,12 @@ acpi_status acpi_enter_sleep_state(u8 sleep_state)
 		return_ACPI_STATUS(AE_AML_OPERAND_VALUE);
 	}
 
-	status = acpi_hw_sleep_dispatch(sleep_state, ACPI_SLEEP_FUNCTION_ID);
+#if !ACPI_REDUCED_HARDWARE
+	if (!acpi_gbl_reduced_hardware)
+		status = acpi_hw_legacy_sleep(sleep_state);
+	else
+#endif
+		status = acpi_hw_extended_sleep(sleep_state);
 	return_ACPI_STATUS(status);
 }
 
@@ -415,8 +353,12 @@ acpi_status acpi_leave_sleep_state_prep(u8 sleep_state)
 
 	ACPI_FUNCTION_TRACE(acpi_leave_sleep_state_prep);
 
-	status =
-	    acpi_hw_sleep_dispatch(sleep_state, ACPI_WAKE_PREP_FUNCTION_ID);
+#if !ACPI_REDUCED_HARDWARE
+	if (!acpi_gbl_reduced_hardware)
+		status = acpi_hw_legacy_wake_prep(sleep_state);
+	else
+#endif
+		status = acpi_hw_extended_wake_prep(sleep_state);
 	return_ACPI_STATUS(status);
 }
 
@@ -440,7 +382,12 @@ acpi_status acpi_leave_sleep_state(u8 sleep_state)
 
 	ACPI_FUNCTION_TRACE(acpi_leave_sleep_state);
 
-	status = acpi_hw_sleep_dispatch(sleep_state, ACPI_WAKE_FUNCTION_ID);
+#if !ACPI_REDUCED_HARDWARE
+	if (!acpi_gbl_reduced_hardware)
+		status = acpi_hw_legacy_wake(sleep_state);
+	else
+#endif
+		status = acpi_hw_extended_wake(sleep_state);
 	return_ACPI_STATUS(status);
 }
 
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index d549e31c6d18..cfcb3abc65b9 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -894,15 +894,6 @@ typedef u8 acpi_adr_space_type;
 #define ACPI_ENABLE_EVENT                       1
 #define ACPI_DISABLE_EVENT                      0
 
-/* Sleep function dispatch */
-
-typedef acpi_status (*acpi_sleep_function) (u8 sleep_state);
-
-struct acpi_sleep_functions {
-	acpi_sleep_function legacy_function;
-	acpi_sleep_function extended_function;
-};
-
 /*
  * External ACPI object definition
  */
-- 
2.11.0

Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Christoph Hellwig]

On Mon, Jun 19, 2017 at 01:56:40PM -0700, Kees Cook wrote:
> Since the ACPICA source is maintained externally to the kernel, we can
> neither switch it to designated initializers nor mark it
> __no_randomize_layout. Until ACPICA-upstream changes[1] land to handle the
> designated initialization, explicitly skip it in the plugin.

NAK.  ACPI has no business rejecting kernel changes to start with, but
independent of that your patch was complete garbage anyway.

There is no need for function pointers here, please include the patch
below instead:

---
>From e8046f6507c2ed60bc501a0c0caa5a3f15f5e3e4 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Sun, 28 May 2017 09:53:45 +0300
Subject: acpi: get rid of acpi_sleep_dispatch

No need for the array of structs of function pointers when we can just
call the handfull of functions directly.

This could be further cleaned up if acpi_gbl_reduced_hardware was defined
true in the ACPI_REDUCED_HARDWARE case, but that's material for the next
round.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/acpi/acpica/hwxfsleep.c | 89 +++++++++--------------------------------
 include/acpi/actypes.h          |  9 -----
 2 files changed, 18 insertions(+), 80 deletions(-)

diff --git a/drivers/acpi/acpica/hwxfsleep.c b/drivers/acpi/acpica/hwxfsleep.c
index 5733b1167e46..66fa3ebddd57 100644
--- a/drivers/acpi/acpica/hwxfsleep.c
+++ b/drivers/acpi/acpica/hwxfsleep.c
@@ -57,26 +57,6 @@ acpi_hw_set_firmware_waking_vector(struct acpi_table_facs *facs,
 				   acpi_physical_address physical_address64);
 #endif
 
-static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id);
-
-/*
- * Dispatch table used to efficiently branch to the various sleep
- * functions.
- */
-#define ACPI_SLEEP_FUNCTION_ID         0
-#define ACPI_WAKE_PREP_FUNCTION_ID     1
-#define ACPI_WAKE_FUNCTION_ID          2
-
-/* Legacy functions are optional, based upon ACPI_REDUCED_HARDWARE */
-
-static struct acpi_sleep_functions acpi_sleep_dispatch[] = {
-	{ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_sleep),
-	 acpi_hw_extended_sleep},
-	{ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake_prep),
-	 acpi_hw_extended_wake_prep},
-	{ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake), acpi_hw_extended_wake}
-};
-
 /*
  * These functions are removed for the ACPI_REDUCED_HARDWARE case:
  *      acpi_set_firmware_waking_vector
@@ -236,53 +216,6 @@ acpi_status acpi_enter_sleep_state_s4bios(void)
 
 ACPI_EXPORT_SYMBOL(acpi_enter_sleep_state_s4bios)
 #endif				/* !ACPI_REDUCED_HARDWARE */
-/*******************************************************************************
- *
- * FUNCTION:    acpi_hw_sleep_dispatch
- *
- * PARAMETERS:  sleep_state         - Which sleep state to enter/exit
- *              function_id         - Sleep, wake_prep, or Wake
- *
- * RETURN:      Status from the invoked sleep handling function.
- *
- * DESCRIPTION: Dispatch a sleep/wake request to the appropriate handling
- *              function.
- *
- ******************************************************************************/
-static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id)
-{
-	acpi_status status;
-	struct acpi_sleep_functions *sleep_functions =
-	    &acpi_sleep_dispatch[function_id];
-
-#if (!ACPI_REDUCED_HARDWARE)
-	/*
-	 * If the Hardware Reduced flag is set (from the FADT), we must
-	 * use the extended sleep registers (FADT). Note: As per the ACPI
-	 * specification, these extended registers are to be used for HW-reduced
-	 * platforms only. They are not general-purpose replacements for the
-	 * legacy PM register sleep support.
-	 */
-	if (acpi_gbl_reduced_hardware) {
-		status = sleep_functions->extended_function(sleep_state);
-	} else {
-		/* Legacy sleep */
-
-		status = sleep_functions->legacy_function(sleep_state);
-	}
-
-	return (status);
-
-#else
-	/*
-	 * For the case where reduced-hardware-only code is being generated,
-	 * we know that only the extended sleep registers are available
-	 */
-	status = sleep_functions->extended_function(sleep_state);
-	return (status);
-
-#endif				/* !ACPI_REDUCED_HARDWARE */
-}
 
 /*******************************************************************************
  *
@@ -389,7 +322,12 @@ acpi_status acpi_enter_sleep_state(u8 sleep_state)
 		return_ACPI_STATUS(AE_AML_OPERAND_VALUE);
 	}
 
-	status = acpi_hw_sleep_dispatch(sleep_state, ACPI_SLEEP_FUNCTION_ID);
+#if !ACPI_REDUCED_HARDWARE
+	if (!acpi_gbl_reduced_hardware)
+		status = acpi_hw_legacy_sleep(sleep_state);
+	else
+#endif
+		status = acpi_hw_extended_sleep(sleep_state);
 	return_ACPI_STATUS(status);
 }
 
@@ -415,8 +353,12 @@ acpi_status acpi_leave_sleep_state_prep(u8 sleep_state)
 
 	ACPI_FUNCTION_TRACE(acpi_leave_sleep_state_prep);
 
-	status =
-	    acpi_hw_sleep_dispatch(sleep_state, ACPI_WAKE_PREP_FUNCTION_ID);
+#if !ACPI_REDUCED_HARDWARE
+	if (!acpi_gbl_reduced_hardware)
+		status = acpi_hw_legacy_wake_prep(sleep_state);
+	else
+#endif
+		status = acpi_hw_extended_wake_prep(sleep_state);
 	return_ACPI_STATUS(status);
 }
 
@@ -440,7 +382,12 @@ acpi_status acpi_leave_sleep_state(u8 sleep_state)
 
 	ACPI_FUNCTION_TRACE(acpi_leave_sleep_state);
 
-	status = acpi_hw_sleep_dispatch(sleep_state, ACPI_WAKE_FUNCTION_ID);
+#if !ACPI_REDUCED_HARDWARE
+	if (!acpi_gbl_reduced_hardware)
+		status = acpi_hw_legacy_wake(sleep_state);
+	else
+#endif
+		status = acpi_hw_extended_wake(sleep_state);
 	return_ACPI_STATUS(status);
 }
 
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index d549e31c6d18..cfcb3abc65b9 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -894,15 +894,6 @@ typedef u8 acpi_adr_space_type;
 #define ACPI_ENABLE_EVENT                       1
 #define ACPI_DISABLE_EVENT                      0
 
-/* Sleep function dispatch */
-
-typedef acpi_status (*acpi_sleep_function) (u8 sleep_state);
-
-struct acpi_sleep_functions {
-	acpi_sleep_function legacy_function;
-	acpi_sleep_function extended_function;
-};
-
 /*
  * External ACPI object definition
  */
-- 
2.11.0

[kernel-hardening] Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Kees Cook]

On Mon, Jun 19, 2017 at 11:56 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Jun 19, 2017 at 01:56:40PM -0700, Kees Cook wrote:
>> Since the ACPICA source is maintained externally to the kernel, we can
>> neither switch it to designated initializers nor mark it
>> __no_randomize_layout. Until ACPICA-upstream changes[1] land to handle the
>> designated initialization, explicitly skip it in the plugin.
>
> NAK.  ACPI has no business rejecting kernel changes to start with, but
> independent of that your patch was complete garbage anyway.

While I don't disagree with your opinion about ACPICA's inclusion in
the kernel, that isn't a battle I want to have. The ACPI maintainers
have a certain way of doing things, and what I need to change is tiny
compared to that.

> There is no need for function pointers here, please include the patch
> below instead:

Can you send the patch to https://github.com/acpica/acpica ? My change
was finally accepted, so this whole issue will go away on the next
refresh. Until then, I don't want to block the entire automatic
structure selection logic of randstruct on a three-function table. :)

Given that it's a tiny exclusion for randstruct, and there is already
a path in motion to fix it, I think this patch is trivial and
sufficient.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Kees Cook]

On Mon, Jun 19, 2017 at 11:56 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Jun 19, 2017 at 01:56:40PM -0700, Kees Cook wrote:
>> Since the ACPICA source is maintained externally to the kernel, we can
>> neither switch it to designated initializers nor mark it
>> __no_randomize_layout. Until ACPICA-upstream changes[1] land to handle the
>> designated initialization, explicitly skip it in the plugin.
>
> NAK.  ACPI has no business rejecting kernel changes to start with, but
> independent of that your patch was complete garbage anyway.

While I don't disagree with your opinion about ACPICA's inclusion in
the kernel, that isn't a battle I want to have. The ACPI maintainers
have a certain way of doing things, and what I need to change is tiny
compared to that.

> There is no need for function pointers here, please include the patch
> below instead:

Can you send the patch to https://github.com/acpica/acpica ? My change
was finally accepted, so this whole issue will go away on the next
refresh. Until then, I don't want to block the entire automatic
structure selection logic of randstruct on a three-function table. :)

Given that it's a tiny exclusion for randstruct, and there is already
a path in motion to fix it, I think this patch is trivial and
sufficient.

-Kees

-- 
Kees Cook
Pixel Security

[kernel-hardening] Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Christoph Hellwig]

On Tue, Jun 20, 2017 at 12:25:53PM -0700, Kees Cook wrote:
> Can you send the patch to https://github.com/acpica/acpica ? My change
> was finally accepted, so this whole issue will go away on the next
> refresh. Until then, I don't want to block the entire automatic
> structure selection logic of randstruct on a three-function table. :)

I do not have a github account and no such thing is required for kernel
development.

I've already CCed the ACPI maintainer the last time I sent the patch,
and I would much prefer if they'd just include it to play silly games.
Ccing them and Linus once again to sort this process mess out.

> Given that it's a tiny exclusion for randstruct, and there is already
> a path in motion to fix it, I think this patch is trivial and
> sufficient.

But it's pointless - just do the right thing.

---
>From e8046f6507c2ed60bc501a0c0caa5a3f15f5e3e4 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Sun, 28 May 2017 09:53:45 +0300
Subject: acpi: get rid of acpi_sleep_dispatch

No need for the array of structs of function pointers when we can just
call the handfull of functions directly.

This could be further cleaned up if acpi_gbl_reduced_hardware was defined
true in the ACPI_REDUCED_HARDWARE case, but that's material for the next
round.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/acpi/acpica/hwxfsleep.c | 89 +++++++++--------------------------------
 include/acpi/actypes.h          |  9 -----
 2 files changed, 18 insertions(+), 80 deletions(-)

diff --git a/drivers/acpi/acpica/hwxfsleep.c b/drivers/acpi/acpica/hwxfsleep.c
index 5733b1167e46..66fa3ebddd57 100644
--- a/drivers/acpi/acpica/hwxfsleep.c
+++ b/drivers/acpi/acpica/hwxfsleep.c
@@ -57,26 +57,6 @@ acpi_hw_set_firmware_waking_vector(struct acpi_table_facs *facs,
 				   acpi_physical_address physical_address64);
 #endif
 
-static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id);
-
-/*
- * Dispatch table used to efficiently branch to the various sleep
- * functions.
- */
-#define ACPI_SLEEP_FUNCTION_ID         0
-#define ACPI_WAKE_PREP_FUNCTION_ID     1
-#define ACPI_WAKE_FUNCTION_ID          2
-
-/* Legacy functions are optional, based upon ACPI_REDUCED_HARDWARE */
-
-static struct acpi_sleep_functions acpi_sleep_dispatch[] = {
-	{ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_sleep),
-	 acpi_hw_extended_sleep},
-	{ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake_prep),
-	 acpi_hw_extended_wake_prep},
-	{ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake), acpi_hw_extended_wake}
-};
-
 /*
  * These functions are removed for the ACPI_REDUCED_HARDWARE case:
  *      acpi_set_firmware_waking_vector
@@ -236,53 +216,6 @@ acpi_status acpi_enter_sleep_state_s4bios(void)
 
 ACPI_EXPORT_SYMBOL(acpi_enter_sleep_state_s4bios)
 #endif				/* !ACPI_REDUCED_HARDWARE */
-/*******************************************************************************
- *
- * FUNCTION:    acpi_hw_sleep_dispatch
- *
- * PARAMETERS:  sleep_state         - Which sleep state to enter/exit
- *              function_id         - Sleep, wake_prep, or Wake
- *
- * RETURN:      Status from the invoked sleep handling function.
- *
- * DESCRIPTION: Dispatch a sleep/wake request to the appropriate handling
- *              function.
- *
- ******************************************************************************/
-static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id)
-{
-	acpi_status status;
-	struct acpi_sleep_functions *sleep_functions =
-	    &acpi_sleep_dispatch[function_id];
-
-#if (!ACPI_REDUCED_HARDWARE)
-	/*
-	 * If the Hardware Reduced flag is set (from the FADT), we must
-	 * use the extended sleep registers (FADT). Note: As per the ACPI
-	 * specification, these extended registers are to be used for HW-reduced
-	 * platforms only. They are not general-purpose replacements for the
-	 * legacy PM register sleep support.
-	 */
-	if (acpi_gbl_reduced_hardware) {
-		status = sleep_functions->extended_function(sleep_state);
-	} else {
-		/* Legacy sleep */
-
-		status = sleep_functions->legacy_function(sleep_state);
-	}
-
-	return (status);
-
-#else
-	/*
-	 * For the case where reduced-hardware-only code is being generated,
-	 * we know that only the extended sleep registers are available
-	 */
-	status = sleep_functions->extended_function(sleep_state);
-	return (status);
-
-#endif				/* !ACPI_REDUCED_HARDWARE */
-}
 
 /*******************************************************************************
  *
@@ -389,7 +322,12 @@ acpi_status acpi_enter_sleep_state(u8 sleep_state)
 		return_ACPI_STATUS(AE_AML_OPERAND_VALUE);
 	}
 
-	status = acpi_hw_sleep_dispatch(sleep_state, ACPI_SLEEP_FUNCTION_ID);
+#if !ACPI_REDUCED_HARDWARE
+	if (!acpi_gbl_reduced_hardware)
+		status = acpi_hw_legacy_sleep(sleep_state);
+	else
+#endif
+		status = acpi_hw_extended_sleep(sleep_state);
 	return_ACPI_STATUS(status);
 }
 
@@ -415,8 +353,12 @@ acpi_status acpi_leave_sleep_state_prep(u8 sleep_state)
 
 	ACPI_FUNCTION_TRACE(acpi_leave_sleep_state_prep);
 
-	status =
-	    acpi_hw_sleep_dispatch(sleep_state, ACPI_WAKE_PREP_FUNCTION_ID);
+#if !ACPI_REDUCED_HARDWARE
+	if (!acpi_gbl_reduced_hardware)
+		status = acpi_hw_legacy_wake_prep(sleep_state);
+	else
+#endif
+		status = acpi_hw_extended_wake_prep(sleep_state);
 	return_ACPI_STATUS(status);
 }
 
@@ -440,7 +382,12 @@ acpi_status acpi_leave_sleep_state(u8 sleep_state)
 
 	ACPI_FUNCTION_TRACE(acpi_leave_sleep_state);
 
-	status = acpi_hw_sleep_dispatch(sleep_state, ACPI_WAKE_FUNCTION_ID);
+#if !ACPI_REDUCED_HARDWARE
+	if (!acpi_gbl_reduced_hardware)
+		status = acpi_hw_legacy_wake(sleep_state);
+	else
+#endif
+		status = acpi_hw_extended_wake(sleep_state);
 	return_ACPI_STATUS(status);
 }
 
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index d549e31c6d18..cfcb3abc65b9 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -894,15 +894,6 @@ typedef u8 acpi_adr_space_type;
 #define ACPI_ENABLE_EVENT                       1
 #define ACPI_DISABLE_EVENT                      0
 
-/* Sleep function dispatch */
-
-typedef acpi_status (*acpi_sleep_function) (u8 sleep_state);
-
-struct acpi_sleep_functions {
-	acpi_sleep_function legacy_function;
-	acpi_sleep_function extended_function;
-};
-
 /*
  * External ACPI object definition
  */
-- 
2.11.0

Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Christoph Hellwig]

On Tue, Jun 20, 2017 at 12:25:53PM -0700, Kees Cook wrote:
> Can you send the patch to https://github.com/acpica/acpica ? My change
> was finally accepted, so this whole issue will go away on the next
> refresh. Until then, I don't want to block the entire automatic
> structure selection logic of randstruct on a three-function table. :)

I do not have a github account and no such thing is required for kernel
development.

I've already CCed the ACPI maintainer the last time I sent the patch,
and I would much prefer if they'd just include it to play silly games.
Ccing them and Linus once again to sort this process mess out.

> Given that it's a tiny exclusion for randstruct, and there is already
> a path in motion to fix it, I think this patch is trivial and
> sufficient.

But it's pointless - just do the right thing.

---
>From e8046f6507c2ed60bc501a0c0caa5a3f15f5e3e4 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Sun, 28 May 2017 09:53:45 +0300
Subject: acpi: get rid of acpi_sleep_dispatch

No need for the array of structs of function pointers when we can just
call the handfull of functions directly.

This could be further cleaned up if acpi_gbl_reduced_hardware was defined
true in the ACPI_REDUCED_HARDWARE case, but that's material for the next
round.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/acpi/acpica/hwxfsleep.c | 89 +++++++++--------------------------------
 include/acpi/actypes.h          |  9 -----
 2 files changed, 18 insertions(+), 80 deletions(-)

diff --git a/drivers/acpi/acpica/hwxfsleep.c b/drivers/acpi/acpica/hwxfsleep.c
index 5733b1167e46..66fa3ebddd57 100644
--- a/drivers/acpi/acpica/hwxfsleep.c
+++ b/drivers/acpi/acpica/hwxfsleep.c
@@ -57,26 +57,6 @@ acpi_hw_set_firmware_waking_vector(struct acpi_table_facs *facs,
 				   acpi_physical_address physical_address64);
 #endif
 
-static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id);
-
-/*
- * Dispatch table used to efficiently branch to the various sleep
- * functions.
- */
-#define ACPI_SLEEP_FUNCTION_ID         0
-#define ACPI_WAKE_PREP_FUNCTION_ID     1
-#define ACPI_WAKE_FUNCTION_ID          2
-
-/* Legacy functions are optional, based upon ACPI_REDUCED_HARDWARE */
-
-static struct acpi_sleep_functions acpi_sleep_dispatch[] = {
-	{ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_sleep),
-	 acpi_hw_extended_sleep},
-	{ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake_prep),
-	 acpi_hw_extended_wake_prep},
-	{ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake), acpi_hw_extended_wake}
-};
-
 /*
  * These functions are removed for the ACPI_REDUCED_HARDWARE case:
  *      acpi_set_firmware_waking_vector
@@ -236,53 +216,6 @@ acpi_status acpi_enter_sleep_state_s4bios(void)
 
 ACPI_EXPORT_SYMBOL(acpi_enter_sleep_state_s4bios)
 #endif				/* !ACPI_REDUCED_HARDWARE */
-/*******************************************************************************
- *
- * FUNCTION:    acpi_hw_sleep_dispatch
- *
- * PARAMETERS:  sleep_state         - Which sleep state to enter/exit
- *              function_id         - Sleep, wake_prep, or Wake
- *
- * RETURN:      Status from the invoked sleep handling function.
- *
- * DESCRIPTION: Dispatch a sleep/wake request to the appropriate handling
- *              function.
- *
- ******************************************************************************/
-static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id)
-{
-	acpi_status status;
-	struct acpi_sleep_functions *sleep_functions =
-	    &acpi_sleep_dispatch[function_id];
-
-#if (!ACPI_REDUCED_HARDWARE)
-	/*
-	 * If the Hardware Reduced flag is set (from the FADT), we must
-	 * use the extended sleep registers (FADT). Note: As per the ACPI
-	 * specification, these extended registers are to be used for HW-reduced
-	 * platforms only. They are not general-purpose replacements for the
-	 * legacy PM register sleep support.
-	 */
-	if (acpi_gbl_reduced_hardware) {
-		status = sleep_functions->extended_function(sleep_state);
-	} else {
-		/* Legacy sleep */
-
-		status = sleep_functions->legacy_function(sleep_state);
-	}
-
-	return (status);
-
-#else
-	/*
-	 * For the case where reduced-hardware-only code is being generated,
-	 * we know that only the extended sleep registers are available
-	 */
-	status = sleep_functions->extended_function(sleep_state);
-	return (status);
-
-#endif				/* !ACPI_REDUCED_HARDWARE */
-}
 
 /*******************************************************************************
  *
@@ -389,7 +322,12 @@ acpi_status acpi_enter_sleep_state(u8 sleep_state)
 		return_ACPI_STATUS(AE_AML_OPERAND_VALUE);
 	}
 
-	status = acpi_hw_sleep_dispatch(sleep_state, ACPI_SLEEP_FUNCTION_ID);
+#if !ACPI_REDUCED_HARDWARE
+	if (!acpi_gbl_reduced_hardware)
+		status = acpi_hw_legacy_sleep(sleep_state);
+	else
+#endif
+		status = acpi_hw_extended_sleep(sleep_state);
 	return_ACPI_STATUS(status);
 }
 
@@ -415,8 +353,12 @@ acpi_status acpi_leave_sleep_state_prep(u8 sleep_state)
 
 	ACPI_FUNCTION_TRACE(acpi_leave_sleep_state_prep);
 
-	status =
-	    acpi_hw_sleep_dispatch(sleep_state, ACPI_WAKE_PREP_FUNCTION_ID);
+#if !ACPI_REDUCED_HARDWARE
+	if (!acpi_gbl_reduced_hardware)
+		status = acpi_hw_legacy_wake_prep(sleep_state);
+	else
+#endif
+		status = acpi_hw_extended_wake_prep(sleep_state);
 	return_ACPI_STATUS(status);
 }
 
@@ -440,7 +382,12 @@ acpi_status acpi_leave_sleep_state(u8 sleep_state)
 
 	ACPI_FUNCTION_TRACE(acpi_leave_sleep_state);
 
-	status = acpi_hw_sleep_dispatch(sleep_state, ACPI_WAKE_FUNCTION_ID);
+#if !ACPI_REDUCED_HARDWARE
+	if (!acpi_gbl_reduced_hardware)
+		status = acpi_hw_legacy_wake(sleep_state);
+	else
+#endif
+		status = acpi_hw_extended_wake(sleep_state);
 	return_ACPI_STATUS(status);
 }
 
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index d549e31c6d18..cfcb3abc65b9 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -894,15 +894,6 @@ typedef u8 acpi_adr_space_type;
 #define ACPI_ENABLE_EVENT                       1
 #define ACPI_DISABLE_EVENT                      0
 
-/* Sleep function dispatch */
-
-typedef acpi_status (*acpi_sleep_function) (u8 sleep_state);
-
-struct acpi_sleep_functions {
-	acpi_sleep_function legacy_function;
-	acpi_sleep_function extended_function;
-};
-
 /*
  * External ACPI object definition
  */
-- 
2.11.0

[kernel-hardening] Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Rafael J. Wysocki]

On Tue, Jun 20, 2017 at 10:35 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Jun 20, 2017 at 12:25:53PM -0700, Kees Cook wrote:
>> Can you send the patch to https://github.com/acpica/acpica ? My change
>> was finally accepted, so this whole issue will go away on the next
>> refresh. Until then, I don't want to block the entire automatic
>> structure selection logic of randstruct on a three-function table. :)
>
> I do not have a github account and no such thing is required for kernel
> development.
>
> I've already CCed the ACPI maintainer the last time I sent the patch,
> and I would much prefer if they'd just include it to play silly games.
> Ccing them and Linus once again to sort this process mess out.
>
>> Given that it's a tiny exclusion for randstruct, and there is already
>> a path in motion to fix it, I think this patch is trivial and
>> sufficient.
>
> But it's pointless - just do the right thing.
>
> ---
> From e8046f6507c2ed60bc501a0c0caa5a3f15f5e3e4 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Sun, 28 May 2017 09:53:45 +0300
> Subject: acpi: get rid of acpi_sleep_dispatch
>
> No need for the array of structs of function pointers when we can just
> call the handfull of functions directly.
>
> This could be further cleaned up if acpi_gbl_reduced_hardware was defined
> true in the ACPI_REDUCED_HARDWARE case, but that's material for the next
> round.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/acpi/acpica/hwxfsleep.c | 89 +++++++++--------------------------------
>  include/acpi/actypes.h          |  9 -----
>  2 files changed, 18 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/acpi/acpica/hwxfsleep.c b/drivers/acpi/acpica/hwxfsleep.c
> index 5733b1167e46..66fa3ebddd57 100644
> --- a/drivers/acpi/acpica/hwxfsleep.c
> +++ b/drivers/acpi/acpica/hwxfsleep.c
> @@ -57,26 +57,6 @@ acpi_hw_set_firmware_waking_vector(struct acpi_table_facs *facs,
>                                    acpi_physical_address physical_address64);
>  #endif
>
> -static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id);
> -
> -/*
> - * Dispatch table used to efficiently branch to the various sleep
> - * functions.
> - */
> -#define ACPI_SLEEP_FUNCTION_ID         0
> -#define ACPI_WAKE_PREP_FUNCTION_ID     1
> -#define ACPI_WAKE_FUNCTION_ID          2
> -
> -/* Legacy functions are optional, based upon ACPI_REDUCED_HARDWARE */
> -
> -static struct acpi_sleep_functions acpi_sleep_dispatch[] = {
> -       {ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_sleep),
> -        acpi_hw_extended_sleep},
> -       {ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake_prep),
> -        acpi_hw_extended_wake_prep},
> -       {ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake), acpi_hw_extended_wake}
> -};
> -
>  /*
>   * These functions are removed for the ACPI_REDUCED_HARDWARE case:
>   *      acpi_set_firmware_waking_vector
> @@ -236,53 +216,6 @@ acpi_status acpi_enter_sleep_state_s4bios(void)
>
>  ACPI_EXPORT_SYMBOL(acpi_enter_sleep_state_s4bios)
>  #endif                         /* !ACPI_REDUCED_HARDWARE */
> -/*******************************************************************************
> - *
> - * FUNCTION:    acpi_hw_sleep_dispatch
> - *
> - * PARAMETERS:  sleep_state         - Which sleep state to enter/exit
> - *              function_id         - Sleep, wake_prep, or Wake
> - *
> - * RETURN:      Status from the invoked sleep handling function.
> - *
> - * DESCRIPTION: Dispatch a sleep/wake request to the appropriate handling
> - *              function.
> - *
> - ******************************************************************************/
> -static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id)
> -{
> -       acpi_status status;
> -       struct acpi_sleep_functions *sleep_functions =
> -           &acpi_sleep_dispatch[function_id];
> -
> -#if (!ACPI_REDUCED_HARDWARE)
> -       /*
> -        * If the Hardware Reduced flag is set (from the FADT), we must
> -        * use the extended sleep registers (FADT). Note: As per the ACPI
> -        * specification, these extended registers are to be used for HW-reduced
> -        * platforms only. They are not general-purpose replacements for the
> -        * legacy PM register sleep support.
> -        */
> -       if (acpi_gbl_reduced_hardware) {
> -               status = sleep_functions->extended_function(sleep_state);
> -       } else {
> -               /* Legacy sleep */
> -
> -               status = sleep_functions->legacy_function(sleep_state);
> -       }
> -
> -       return (status);
> -
> -#else
> -       /*
> -        * For the case where reduced-hardware-only code is being generated,
> -        * we know that only the extended sleep registers are available
> -        */
> -       status = sleep_functions->extended_function(sleep_state);
> -       return (status);
> -
> -#endif                         /* !ACPI_REDUCED_HARDWARE */
> -}
>
>  /*******************************************************************************
>   *
> @@ -389,7 +322,12 @@ acpi_status acpi_enter_sleep_state(u8 sleep_state)
>                 return_ACPI_STATUS(AE_AML_OPERAND_VALUE);
>         }
>
> -       status = acpi_hw_sleep_dispatch(sleep_state, ACPI_SLEEP_FUNCTION_ID);
> +#if !ACPI_REDUCED_HARDWARE
> +       if (!acpi_gbl_reduced_hardware)
> +               status = acpi_hw_legacy_sleep(sleep_state);
> +       else
> +#endif
> +               status = acpi_hw_extended_sleep(sleep_state);
>         return_ACPI_STATUS(status);
>  }
>

I guess you can avoid these #ifs in function bodies?

Thanks,
Rafael

Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Rafael J. Wysocki]

On Tue, Jun 20, 2017 at 10:35 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Jun 20, 2017 at 12:25:53PM -0700, Kees Cook wrote:
>> Can you send the patch to https://github.com/acpica/acpica ? My change
>> was finally accepted, so this whole issue will go away on the next
>> refresh. Until then, I don't want to block the entire automatic
>> structure selection logic of randstruct on a three-function table. :)
>
> I do not have a github account and no such thing is required for kernel
> development.
>
> I've already CCed the ACPI maintainer the last time I sent the patch,
> and I would much prefer if they'd just include it to play silly games.
> Ccing them and Linus once again to sort this process mess out.
>
>> Given that it's a tiny exclusion for randstruct, and there is already
>> a path in motion to fix it, I think this patch is trivial and
>> sufficient.
>
> But it's pointless - just do the right thing.
>
> ---
> From e8046f6507c2ed60bc501a0c0caa5a3f15f5e3e4 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Sun, 28 May 2017 09:53:45 +0300
> Subject: acpi: get rid of acpi_sleep_dispatch
>
> No need for the array of structs of function pointers when we can just
> call the handfull of functions directly.
>
> This could be further cleaned up if acpi_gbl_reduced_hardware was defined
> true in the ACPI_REDUCED_HARDWARE case, but that's material for the next
> round.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/acpi/acpica/hwxfsleep.c | 89 +++++++++--------------------------------
>  include/acpi/actypes.h          |  9 -----
>  2 files changed, 18 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/acpi/acpica/hwxfsleep.c b/drivers/acpi/acpica/hwxfsleep.c
> index 5733b1167e46..66fa3ebddd57 100644
> --- a/drivers/acpi/acpica/hwxfsleep.c
> +++ b/drivers/acpi/acpica/hwxfsleep.c
> @@ -57,26 +57,6 @@ acpi_hw_set_firmware_waking_vector(struct acpi_table_facs *facs,
>                                    acpi_physical_address physical_address64);
>  #endif
>
> -static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id);
> -
> -/*
> - * Dispatch table used to efficiently branch to the various sleep
> - * functions.
> - */
> -#define ACPI_SLEEP_FUNCTION_ID         0
> -#define ACPI_WAKE_PREP_FUNCTION_ID     1
> -#define ACPI_WAKE_FUNCTION_ID          2
> -
> -/* Legacy functions are optional, based upon ACPI_REDUCED_HARDWARE */
> -
> -static struct acpi_sleep_functions acpi_sleep_dispatch[] = {
> -       {ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_sleep),
> -        acpi_hw_extended_sleep},
> -       {ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake_prep),
> -        acpi_hw_extended_wake_prep},
> -       {ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake), acpi_hw_extended_wake}
> -};
> -
>  /*
>   * These functions are removed for the ACPI_REDUCED_HARDWARE case:
>   *      acpi_set_firmware_waking_vector
> @@ -236,53 +216,6 @@ acpi_status acpi_enter_sleep_state_s4bios(void)
>
>  ACPI_EXPORT_SYMBOL(acpi_enter_sleep_state_s4bios)
>  #endif                         /* !ACPI_REDUCED_HARDWARE */
> -/*******************************************************************************
> - *
> - * FUNCTION:    acpi_hw_sleep_dispatch
> - *
> - * PARAMETERS:  sleep_state         - Which sleep state to enter/exit
> - *              function_id         - Sleep, wake_prep, or Wake
> - *
> - * RETURN:      Status from the invoked sleep handling function.
> - *
> - * DESCRIPTION: Dispatch a sleep/wake request to the appropriate handling
> - *              function.
> - *
> - ******************************************************************************/
> -static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id)
> -{
> -       acpi_status status;
> -       struct acpi_sleep_functions *sleep_functions =
> -           &acpi_sleep_dispatch[function_id];
> -
> -#if (!ACPI_REDUCED_HARDWARE)
> -       /*
> -        * If the Hardware Reduced flag is set (from the FADT), we must
> -        * use the extended sleep registers (FADT). Note: As per the ACPI
> -        * specification, these extended registers are to be used for HW-reduced
> -        * platforms only. They are not general-purpose replacements for the
> -        * legacy PM register sleep support.
> -        */
> -       if (acpi_gbl_reduced_hardware) {
> -               status = sleep_functions->extended_function(sleep_state);
> -       } else {
> -               /* Legacy sleep */
> -
> -               status = sleep_functions->legacy_function(sleep_state);
> -       }
> -
> -       return (status);
> -
> -#else
> -       /*
> -        * For the case where reduced-hardware-only code is being generated,
> -        * we know that only the extended sleep registers are available
> -        */
> -       status = sleep_functions->extended_function(sleep_state);
> -       return (status);
> -
> -#endif                         /* !ACPI_REDUCED_HARDWARE */
> -}
>
>  /*******************************************************************************
>   *
> @@ -389,7 +322,12 @@ acpi_status acpi_enter_sleep_state(u8 sleep_state)
>                 return_ACPI_STATUS(AE_AML_OPERAND_VALUE);
>         }
>
> -       status = acpi_hw_sleep_dispatch(sleep_state, ACPI_SLEEP_FUNCTION_ID);
> +#if !ACPI_REDUCED_HARDWARE
> +       if (!acpi_gbl_reduced_hardware)
> +               status = acpi_hw_legacy_sleep(sleep_state);
> +       else
> +#endif
> +               status = acpi_hw_extended_sleep(sleep_state);
>         return_ACPI_STATUS(status);
>  }
>

I guess you can avoid these #ifs in function bodies?

Thanks,
Rafael

[kernel-hardening] Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Rafael J. Wysocki]

On Tue, Jun 20, 2017 at 10:35 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Jun 20, 2017 at 12:25:53PM -0700, Kees Cook wrote:
>> Can you send the patch to https://github.com/acpica/acpica ? My change
>> was finally accepted, so this whole issue will go away on the next
>> refresh. Until then, I don't want to block the entire automatic
>> structure selection logic of randstruct on a three-function table. :)
>
> I do not have a github account and no such thing is required for kernel
> development.

It isn't required for the ACPICA material either.

You just need to CC the ACPICA maintainers, as per MAINTAINERS, on
your ACPICA patches.  They pick up stuff that looks good to them.

And we tend to prefer routing ACPICA changes through the upstream,
because failing to do so usually turns out to be painful in the long
term.  I don't think it is unreasonable to ask for cooperation in that
respect.

Thanks,
Rafael

Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Rafael J. Wysocki]

On Tue, Jun 20, 2017 at 10:35 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Jun 20, 2017 at 12:25:53PM -0700, Kees Cook wrote:
>> Can you send the patch to https://github.com/acpica/acpica ? My change
>> was finally accepted, so this whole issue will go away on the next
>> refresh. Until then, I don't want to block the entire automatic
>> structure selection logic of randstruct on a three-function table. :)
>
> I do not have a github account and no such thing is required for kernel
> development.

It isn't required for the ACPICA material either.

You just need to CC the ACPICA maintainers, as per MAINTAINERS, on
your ACPICA patches.  They pick up stuff that looks good to them.

And we tend to prefer routing ACPICA changes through the upstream,
because failing to do so usually turns out to be painful in the long
term.  I don't think it is unreasonable to ask for cooperation in that
respect.

Thanks,
Rafael

[kernel-hardening] Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Kees Cook]

On Tue, Jun 20, 2017 at 2:34 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Tue, Jun 20, 2017 at 10:35 PM, Christoph Hellwig <hch@infradead.org> wrote:
>> On Tue, Jun 20, 2017 at 12:25:53PM -0700, Kees Cook wrote:
>>> Can you send the patch to https://github.com/acpica/acpica ? My change
>>> was finally accepted, so this whole issue will go away on the next
>>> refresh. Until then, I don't want to block the entire automatic
>>> structure selection logic of randstruct on a three-function table. :)
>>
>> I do not have a github account and no such thing is required for kernel
>> development.
>
> It isn't required for the ACPICA material either.
>
> You just need to CC the ACPICA maintainers, as per MAINTAINERS, on
> your ACPICA patches.  They pick up stuff that looks good to them.
>
> And we tend to prefer routing ACPICA changes through the upstream,
> because failing to do so usually turns out to be painful in the long
> term.  I don't think it is unreasonable to ask for cooperation in that
> respect.

I'd like to unblock randstruct, so what's the easiest way to move
this? My version of changes have already landed upstream in ACPICA,
but I don't know how frequently they get flushed back into the kernel.

I can't turn on randstruct auto-selection in -next without either
ACPICA using (or not needing) designated initializers or just
blacklisting it in the randstruct plugin itself. I would much prefer
the latter as the problem is solved in ACPICA upstream already but
just isn't in the kernel yet, and I want to get testing of the
auto-selection ASAP. Once it's in the kernel I can drop the blacklist.

Christoph: how about a middle ground where randstruct blacklists
ACPICA in -next and if ACPICA is fixed by the time the merge window
opens, I'll drop the blacklist. That gets the testing coverage without
what you see as an ugly hack right now. I just really don't want to
waste any more time on this since there are SO many other randomized
structures I'd like to be sure are getting testing.

Alternatively, if the ACPICA folks Ack Christoph's patch, I can carry
that in the randstruct tree for -next instead?

Thanks,

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Kees Cook]

On Tue, Jun 20, 2017 at 2:34 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Tue, Jun 20, 2017 at 10:35 PM, Christoph Hellwig <hch@infradead.org> wrote:
>> On Tue, Jun 20, 2017 at 12:25:53PM -0700, Kees Cook wrote:
>>> Can you send the patch to https://github.com/acpica/acpica ? My change
>>> was finally accepted, so this whole issue will go away on the next
>>> refresh. Until then, I don't want to block the entire automatic
>>> structure selection logic of randstruct on a three-function table. :)
>>
>> I do not have a github account and no such thing is required for kernel
>> development.
>
> It isn't required for the ACPICA material either.
>
> You just need to CC the ACPICA maintainers, as per MAINTAINERS, on
> your ACPICA patches.  They pick up stuff that looks good to them.
>
> And we tend to prefer routing ACPICA changes through the upstream,
> because failing to do so usually turns out to be painful in the long
> term.  I don't think it is unreasonable to ask for cooperation in that
> respect.

I'd like to unblock randstruct, so what's the easiest way to move
this? My version of changes have already landed upstream in ACPICA,
but I don't know how frequently they get flushed back into the kernel.

I can't turn on randstruct auto-selection in -next without either
ACPICA using (or not needing) designated initializers or just
blacklisting it in the randstruct plugin itself. I would much prefer
the latter as the problem is solved in ACPICA upstream already but
just isn't in the kernel yet, and I want to get testing of the
auto-selection ASAP. Once it's in the kernel I can drop the blacklist.

Christoph: how about a middle ground where randstruct blacklists
ACPICA in -next and if ACPICA is fixed by the time the merge window
opens, I'll drop the blacklist. That gets the testing coverage without
what you see as an ugly hack right now. I just really don't want to
waste any more time on this since there are SO many other randomized
structures I'd like to be sure are getting testing.

Alternatively, if the ACPICA folks Ack Christoph's patch, I can carry
that in the randstruct tree for -next instead?

Thanks,

-Kees

-- 
Kees Cook
Pixel Security

[kernel-hardening] Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Rafael J. Wysocki]

On Thursday, June 22, 2017 04:57:39 PM Kees Cook wrote:
> On Tue, Jun 20, 2017 at 2:34 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > On Tue, Jun 20, 2017 at 10:35 PM, Christoph Hellwig <hch@infradead.org> wrote:
> >> On Tue, Jun 20, 2017 at 12:25:53PM -0700, Kees Cook wrote:
> >>> Can you send the patch to https://github.com/acpica/acpica ? My change
> >>> was finally accepted, so this whole issue will go away on the next
> >>> refresh. Until then, I don't want to block the entire automatic
> >>> structure selection logic of randstruct on a three-function table. :)
> >>
> >> I do not have a github account and no such thing is required for kernel
> >> development.
> >
> > It isn't required for the ACPICA material either.
> >
> > You just need to CC the ACPICA maintainers, as per MAINTAINERS, on
> > your ACPICA patches.  They pick up stuff that looks good to them.
> >
> > And we tend to prefer routing ACPICA changes through the upstream,
> > because failing to do so usually turns out to be painful in the long
> > term.  I don't think it is unreasonable to ask for cooperation in that
> > respect.
> 
> I'd like to unblock randstruct, so what's the easiest way to move
> this? My version of changes have already landed upstream in ACPICA,
> but I don't know how frequently they get flushed back into the kernel.

Usually, when there's a new ACPICA release, but occasionally that happens
faster.

Which commit in upstream ACPICA is this?

> I can't turn on randstruct auto-selection in -next without either
> ACPICA using (or not needing) designated initializers or just
> blacklisting it in the randstruct plugin itself. I would much prefer
> the latter as the problem is solved in ACPICA upstream already but
> just isn't in the kernel yet, and I want to get testing of the
> auto-selection ASAP. Once it's in the kernel I can drop the blacklist.
> 
> Christoph: how about a middle ground where randstruct blacklists
> ACPICA in -next and if ACPICA is fixed by the time the merge window
> opens, I'll drop the blacklist. That gets the testing coverage without
> what you see as an ugly hack right now. I just really don't want to
> waste any more time on this since there are SO many other randomized
> structures I'd like to be sure are getting testing.
> 
> Alternatively, if the ACPICA folks Ack Christoph's patch, I can carry
> that in the randstruct tree for -next instead?

Maybe we can simply forward port the ACPICA commit right away.

Lv, can you take care of this, please?

Thanks,
Rafael

Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Rafael J. Wysocki]

On Thursday, June 22, 2017 04:57:39 PM Kees Cook wrote:
> On Tue, Jun 20, 2017 at 2:34 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > On Tue, Jun 20, 2017 at 10:35 PM, Christoph Hellwig <hch@infradead.org> wrote:
> >> On Tue, Jun 20, 2017 at 12:25:53PM -0700, Kees Cook wrote:
> >>> Can you send the patch to https://github.com/acpica/acpica ? My change
> >>> was finally accepted, so this whole issue will go away on the next
> >>> refresh. Until then, I don't want to block the entire automatic
> >>> structure selection logic of randstruct on a three-function table. :)
> >>
> >> I do not have a github account and no such thing is required for kernel
> >> development.
> >
> > It isn't required for the ACPICA material either.
> >
> > You just need to CC the ACPICA maintainers, as per MAINTAINERS, on
> > your ACPICA patches.  They pick up stuff that looks good to them.
> >
> > And we tend to prefer routing ACPICA changes through the upstream,
> > because failing to do so usually turns out to be painful in the long
> > term.  I don't think it is unreasonable to ask for cooperation in that
> > respect.
> 
> I'd like to unblock randstruct, so what's the easiest way to move
> this? My version of changes have already landed upstream in ACPICA,
> but I don't know how frequently they get flushed back into the kernel.

Usually, when there's a new ACPICA release, but occasionally that happens
faster.

Which commit in upstream ACPICA is this?

> I can't turn on randstruct auto-selection in -next without either
> ACPICA using (or not needing) designated initializers or just
> blacklisting it in the randstruct plugin itself. I would much prefer
> the latter as the problem is solved in ACPICA upstream already but
> just isn't in the kernel yet, and I want to get testing of the
> auto-selection ASAP. Once it's in the kernel I can drop the blacklist.
> 
> Christoph: how about a middle ground where randstruct blacklists
> ACPICA in -next and if ACPICA is fixed by the time the merge window
> opens, I'll drop the blacklist. That gets the testing coverage without
> what you see as an ugly hack right now. I just really don't want to
> waste any more time on this since there are SO many other randomized
> structures I'd like to be sure are getting testing.
> 
> Alternatively, if the ACPICA folks Ack Christoph's patch, I can carry
> that in the randstruct tree for -next instead?

Maybe we can simply forward port the ACPICA commit right away.

Lv, can you take care of this, please?

Thanks,
Rafael

[kernel-hardening] Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Kees Cook]

On Thu, Jun 22, 2017 at 4:59 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Thursday, June 22, 2017 04:57:39 PM Kees Cook wrote:
>> On Tue, Jun 20, 2017 at 2:34 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> > On Tue, Jun 20, 2017 at 10:35 PM, Christoph Hellwig <hch@infradead.org> wrote:
>> >> On Tue, Jun 20, 2017 at 12:25:53PM -0700, Kees Cook wrote:
>> >>> Can you send the patch to https://github.com/acpica/acpica ? My change
>> >>> was finally accepted, so this whole issue will go away on the next
>> >>> refresh. Until then, I don't want to block the entire automatic
>> >>> structure selection logic of randstruct on a three-function table. :)
>> >>
>> >> I do not have a github account and no such thing is required for kernel
>> >> development.
>> >
>> > It isn't required for the ACPICA material either.
>> >
>> > You just need to CC the ACPICA maintainers, as per MAINTAINERS, on
>> > your ACPICA patches.  They pick up stuff that looks good to them.
>> >
>> > And we tend to prefer routing ACPICA changes through the upstream,
>> > because failing to do so usually turns out to be painful in the long
>> > term.  I don't think it is unreasonable to ask for cooperation in that
>> > respect.
>>
>> I'd like to unblock randstruct, so what's the easiest way to move
>> this? My version of changes have already landed upstream in ACPICA,
>> but I don't know how frequently they get flushed back into the kernel.
>
> Usually, when there's a new ACPICA release, but occasionally that happens
> faster.
>
> Which commit in upstream ACPICA is this?

https://github.com/acpica/acpica/commit/2058b3bf5deecb9644d676703bd97d1bce5e612a

>> I can't turn on randstruct auto-selection in -next without either
>> ACPICA using (or not needing) designated initializers or just
>> blacklisting it in the randstruct plugin itself. I would much prefer
>> the latter as the problem is solved in ACPICA upstream already but
>> just isn't in the kernel yet, and I want to get testing of the
>> auto-selection ASAP. Once it's in the kernel I can drop the blacklist.
>>
>> Christoph: how about a middle ground where randstruct blacklists
>> ACPICA in -next and if ACPICA is fixed by the time the merge window
>> opens, I'll drop the blacklist. That gets the testing coverage without
>> what you see as an ugly hack right now. I just really don't want to
>> waste any more time on this since there are SO many other randomized
>> structures I'd like to be sure are getting testing.
>>
>> Alternatively, if the ACPICA folks Ack Christoph's patch, I can carry
>> that in the randstruct tree for -next instead?
>
> Maybe we can simply forward port the ACPICA commit right away.
>
> Lv, can you take care of this, please?

That would be great, thanks!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 3/4] randstruct: Disable randomization of ACPICA structs

[Kees Cook]

On Thu, Jun 22, 2017 at 4:59 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Thursday, June 22, 2017 04:57:39 PM Kees Cook wrote:
>> On Tue, Jun 20, 2017 at 2:34 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> > On Tue, Jun 20, 2017 at 10:35 PM, Christoph Hellwig <hch@infradead.org> wrote:
>> >> On Tue, Jun 20, 2017 at 12:25:53PM -0700, Kees Cook wrote:
>> >>> Can you send the patch to https://github.com/acpica/acpica ? My change
>> >>> was finally accepted, so this whole issue will go away on the next
>> >>> refresh. Until then, I don't want to block the entire automatic
>> >>> structure selection logic of randstruct on a three-function table. :)
>> >>
>> >> I do not have a github account and no such thing is required for kernel
>> >> development.
>> >
>> > It isn't required for the ACPICA material either.
>> >
>> > You just need to CC the ACPICA maintainers, as per MAINTAINERS, on
>> > your ACPICA patches.  They pick up stuff that looks good to them.
>> >
>> > And we tend to prefer routing ACPICA changes through the upstream,
>> > because failing to do so usually turns out to be painful in the long
>> > term.  I don't think it is unreasonable to ask for cooperation in that
>> > respect.
>>
>> I'd like to unblock randstruct, so what's the easiest way to move
>> this? My version of changes have already landed upstream in ACPICA,
>> but I don't know how frequently they get flushed back into the kernel.
>
> Usually, when there's a new ACPICA release, but occasionally that happens
> faster.
>
> Which commit in upstream ACPICA is this?

https://github.com/acpica/acpica/commit/2058b3bf5deecb9644d676703bd97d1bce5e612a

>> I can't turn on randstruct auto-selection in -next without either
>> ACPICA using (or not needing) designated initializers or just
>> blacklisting it in the randstruct plugin itself. I would much prefer
>> the latter as the problem is solved in ACPICA upstream already but
>> just isn't in the kernel yet, and I want to get testing of the
>> auto-selection ASAP. Once it's in the kernel I can drop the blacklist.
>>
>> Christoph: how about a middle ground where randstruct blacklists
>> ACPICA in -next and if ACPICA is fixed by the time the merge window
>> opens, I'll drop the blacklist. That gets the testing coverage without
>> what you see as an ugly hack right now. I just really don't want to
>> waste any more time on this since there are SO many other randomized
>> structures I'd like to be sure are getting testing.
>>
>> Alternatively, if the ACPICA folks Ack Christoph's patch, I can carry
>> that in the randstruct tree for -next instead?
>
> Maybe we can simply forward port the ACPICA commit right away.
>
> Lv, can you take care of this, please?

That would be great, thanks!

-Kees

-- 
Kees Cook
Pixel Security

[kernel-hardening] [PATCH 4/4] randstruct: Enable function pointer struct detection

[Kees Cook]

This enables the automatic structure selection logic in the randstruct
GCC plugin. The selection logic randomizes all structures that contain
only function pointers, unless marked with __no_randomize_layout.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/Kconfig                                  | 12 +++++++-----
 scripts/gcc-plugins/randomize_layout_plugin.c |  3 ---
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 810bf206f221..d8e57d6216b4 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -448,11 +448,13 @@ config GCC_PLUGIN_RANDSTRUCT
 	depends on GCC_PLUGINS
 	select MODVERSIONS if MODULES
 	help
-	  If you say Y here, the layouts of structures explicitly
-	  marked by __randomize_layout will be randomized at
-	  compile-time.  This can introduce the requirement of an
-	  additional information exposure vulnerability for exploits
-	  targeting these structure types.
+	  If you say Y here, the layouts of structures that are entirely
+	  function pointers (and have not been manually annotated with
+	  __no_randomize_layout), or structures that have been explicitly
+	  marked with __randomize_layout, will be randomized at compile-time.
+	  This can introduce the requirement of an additional information
+	  exposure vulnerability for exploits targeting these structure
+	  types.
 
 	  Enabling this feature will introduce some performance impact,
 	  slightly increase memory usage, and prevent the use of forensic
diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
index e6e02a40d522..bb2c6789c4b7 100644
--- a/scripts/gcc-plugins/randomize_layout_plugin.c
+++ b/scripts/gcc-plugins/randomize_layout_plugin.c
@@ -437,9 +437,6 @@ static int is_pure_ops_struct(const_tree node)
 
 	gcc_assert(TREE_CODE(node) == RECORD_TYPE || TREE_CODE(node) == UNION_TYPE);
 
-	/* XXX: Do not apply randomization to all-ftpr structs yet. */
-	return 0;
-
 	for (field = TYPE_FIELDS(node); field; field = TREE_CHAIN(field)) {
 		const_tree fieldtype = get_field_type(field);
 		enum tree_code code = TREE_CODE(fieldtype);
-- 
2.7.4

[PATCH 4/4] randstruct: Enable function pointer struct detection

[Kees Cook]

This enables the automatic structure selection logic in the randstruct
GCC plugin. The selection logic randomizes all structures that contain
only function pointers, unless marked with __no_randomize_layout.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/Kconfig                                  | 12 +++++++-----
 scripts/gcc-plugins/randomize_layout_plugin.c |  3 ---
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 810bf206f221..d8e57d6216b4 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -448,11 +448,13 @@ config GCC_PLUGIN_RANDSTRUCT
 	depends on GCC_PLUGINS
 	select MODVERSIONS if MODULES
 	help
-	  If you say Y here, the layouts of structures explicitly
-	  marked by __randomize_layout will be randomized at
-	  compile-time.  This can introduce the requirement of an
-	  additional information exposure vulnerability for exploits
-	  targeting these structure types.
+	  If you say Y here, the layouts of structures that are entirely
+	  function pointers (and have not been manually annotated with
+	  __no_randomize_layout), or structures that have been explicitly
+	  marked with __randomize_layout, will be randomized at compile-time.
+	  This can introduce the requirement of an additional information
+	  exposure vulnerability for exploits targeting these structure
+	  types.
 
 	  Enabling this feature will introduce some performance impact,
 	  slightly increase memory usage, and prevent the use of forensic
diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
index e6e02a40d522..bb2c6789c4b7 100644
--- a/scripts/gcc-plugins/randomize_layout_plugin.c
+++ b/scripts/gcc-plugins/randomize_layout_plugin.c
@@ -437,9 +437,6 @@ static int is_pure_ops_struct(const_tree node)
 
 	gcc_assert(TREE_CODE(node) == RECORD_TYPE || TREE_CODE(node) == UNION_TYPE);
 
-	/* XXX: Do not apply randomization to all-ftpr structs yet. */
-	return 0;
-
 	for (field = TYPE_FIELDS(node); field; field = TREE_CHAIN(field)) {
 		const_tree fieldtype = get_field_type(field);
 		enum tree_code code = TREE_CODE(fieldtype);
-- 
2.7.4