Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1651608 > unrolled thread

[PATCH v2 00/20] Introduce struct layout randomization plugin

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/20] Introduce struct layout randomization plugin Kees Cook <keescook@chromium.org> - 2017-05-27 03:20 +0200
    [PATCH v2 06/20] randstruct: Whitelist UNIXCB cast Kees Cook <keescook@chromium.org> - 2017-05-27 03:20 +0200
      Re: [PATCH v2 06/20] randstruct: Whitelist UNIXCB cast Kees Cook <keescook@chromium.org> - 2017-05-27 03:50 +0200
      Re: [PATCH v2 06/20] randstruct: Whitelist UNIXCB cast Christoph Hellwig <hch@infradead.org> - 2017-05-28 10:00 +0200
    [PATCH v2 13/20] drm/amdgpu: Use designated initializers Kees Cook <keescook@chromium.org> - 2017-05-27 03:20 +0200
    [PATCH v2 05/20] randstruct: Whitelist struct security_hook_heads cast Kees Cook <keescook@chromium.org> - 2017-05-27 03:20 +0200
      Re: [PATCH v2 05/20] randstruct: Whitelist struct  security_hook_heads cast Christoph Hellwig <hch@infradead.org> - 2017-05-27 10:50 +0200
        Re: [PATCH v2 05/20] randstruct: Whitelist struct security_hook_heads cast Kees Cook <keescook@chromium.org> - 2017-05-27 22:20 +0200
          Re: [PATCH v2 05/20] randstruct: Whitelist struct security_hook_heads cast Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2017-05-28 00:10 +0200
            Re: [kernel-hardening] Re: [PATCH v2 05/20] randstruct: Whitelist  struct security_hook_heads cast Kees Cook <keescook@chromium.org> - 2017-05-28 02:50 +0200
          Re: [PATCH v2 05/20] randstruct: Whitelist struct security_hook_heads  cast James Morris <jmorris@namei.org> - 2017-05-30 12:40 +0200
    [PATCH v2 02/20] gcc-plugins: Detail c-common.h location for GCC 4.6 Kees Cook <keescook@chromium.org> - 2017-05-27 03:20 +0200
    [PATCH v2 11/20] randstruct: Disable randomization of ACPICA structs Kees Cook <keescook@chromium.org> - 2017-05-27 03:20 +0200
      Re: [PATCH v2 11/20] randstruct: Disable randomization of ACPICA  structs Christoph Hellwig <hch@infradead.org> - 2017-05-27 10:50 +0200
        Re: [PATCH v2 11/20] randstruct: Disable randomization of ACPICA structs Kees Cook <keescook@chromium.org> - 2017-05-27 22:10 +0200
          Re: [PATCH v2 11/20] randstruct: Disable randomization of ACPICA  structs Christoph Hellwig <hch@infradead.org> - 2017-05-28 07:00 +0200
    Re: [PATCH v2 19/20] [RFC] task_struct: Allow randomized layout Kees Cook <keescook@chromium.org> - 2017-05-27 03:30 +0200
    Re: [PATCH v2 19/20] [RFC] task_struct: Allow randomized layout Linus Torvalds <torvalds@linux-foundation.org> - 2017-05-27 03:30 +0200
    [PATCH v2 15/20] mtk-vcodec: Use designated initializers Kees Cook <keescook@chromium.org> - 2017-05-27 03:50 +0200
    [PATCH v2 14/20] drm/amd/powerplay: Use designated initializers Kees Cook <keescook@chromium.org> - 2017-05-27 03:50 +0200
      Re: [PATCH v2 14/20] drm/amd/powerplay: Use designated initializers Christoph Hellwig <hch@infradead.org> - 2017-05-27 10:50 +0200
        Re: [PATCH v2 14/20] drm/amd/powerplay: Use designated initializers Kees Cook <keescook@chromium.org> - 2017-05-27 22:20 +0200
    [PATCH v2 20/20] ACPICA: Use designated initializers Kees Cook <keescook@chromium.org> - 2017-05-27 03:50 +0200
      Re: [PATCH v2 20/20] ACPICA: Use designated initializers Christoph Hellwig <hch@infradead.org> - 2017-05-28 09:50 +0200
    [PATCH v2 18/20] randstruct: Enable function pointer struct detection Kees Cook <keescook@chromium.org> - 2017-05-27 03:50 +0200
    [PATCH v2 16/20] ntfs: Use ERR_CAST() to avoid cross-structure cast Kees Cook <keescook@chromium.org> - 2017-05-27 03:50 +0200
    [PATCH v2 17/20] ocfs2: Use ERR_CAST() to avoid cross-structure cast Kees Cook <keescook@chromium.org> - 2017-05-27 03:50 +0200
    [PATCH v2 19/20] [RFC] task_struct: Allow randomized layout Kees Cook <keescook@chromium.org> - 2017-05-27 03:50 +0200
    [PATCH v2 10/20] randstruct: opt-out externally exposed function pointer structs Kees Cook <keescook@chromium.org> - 2017-05-27 04:00 +0200
    [PATCH v2 12/20] sgi-xp: Use designated initializers Kees Cook <keescook@chromium.org> - 2017-05-27 04:00 +0200
      Re: [PATCH v2 12/20] sgi-xp: Use designated initializers Christoph Hellwig <hch@infradead.org> - 2017-05-27 10:50 +0200
    [PATCH v2 01/20] NFS: Avoid cross-structure casting Kees Cook <keescook@chromium.org> - 2017-05-27 04:00 +0200
      Re: [PATCH v2 01/20] NFS: Avoid cross-structure casting Christoph Hellwig <hch@infradead.org> - 2017-05-28 10:00 +0200
        Re: [PATCH v2 01/20] NFS: Avoid cross-structure casting Kees Cook <keescook@chromium.org> - 2017-05-28 19:00 +0200
    [PATCH v2 03/20] compiler: Add __designated_init annotation Kees Cook <keescook@chromium.org> - 2017-05-27 04:00 +0200
    [PATCH v2 08/20] randstruct: Whitelist NIU struct page overloading Kees Cook <keescook@chromium.org> - 2017-05-27 04:00 +0200
      Re: [PATCH v2 08/20] randstruct: Whitelist NIU struct page  overloading Christoph Hellwig <hch@infradead.org> - 2017-05-28 10:20 +0200
        Re: [PATCH v2 08/20] randstruct: Whitelist NIU struct page overloading Kees Cook <keescook@chromium.org> - 2017-05-28 19:40 +0200
        Re: [PATCH v2 08/20] randstruct: Whitelist NIU struct page overloading Kees Cook <keescook@chromium.org> - 2017-05-28 19:40 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1651815 — Re: [PATCH v2 14/20] drm/amd/powerplay: Use designated initializers

> +	{ .isFunctionNeededInRuntimeTable = NULL,
> +	  .tableFunction = tf_vega10_thermal_initialize },

I'd say drop the NULL initializations as they are implicit anyway.

[toc] | [prev] | [next] | [standalone]

#1651980 — Re: [PATCH v2 14/20] drm/amd/powerplay: Use designated initializers

On Sat, May 27, 2017 at 1:47 AM, Christoph Hellwig <hch@infradead.org> wrote:
>> +     { .isFunctionNeededInRuntimeTable = NULL,
>> +       .tableFunction = tf_vega10_thermal_initialize },
>
> I'd say drop the NULL initializations as they are implicit anyway.

Oh, dur, yeah, I'll improve this.

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1651711 — [PATCH v2 20/20] ACPICA: Use designated initializers

The struct layout randomization plugin detects and randomizes any structs
that contain only function pointers. Once layout is randomized, all
initialization must be designated or the compiler will misalign the
assignments. This switches all the ACPICA function pointer struct to
use designated initializers, using the proposed upstream ACPICA macro:
https://github.com/acpica/acpica/pull/248/

Cc: Bob Moore <robert.moore@intel.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/acpi/acpica/hwxfsleep.c               | 14 +++++++++-----
 include/acpi/platform/acenv.h                 |  4 ++++
 include/acpi/platform/aclinux.h               |  2 ++
 scripts/gcc-plugins/randomize_layout_plugin.c |  4 ----
 4 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/acpica/hwxfsleep.c b/drivers/acpi/acpica/hwxfsleep.c
index 5733b1167e46..558fe4cdf29a 100644
--- a/drivers/acpi/acpica/hwxfsleep.c
+++ b/drivers/acpi/acpica/hwxfsleep.c
@@ -70,11 +70,15 @@ static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id);
 /* 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}
+	{ACPI_STRUCT_INIT (legacy_function,
+			   ACPI_HW_OPTIONAL_FUNCTION (acpi_hw_legacy_sleep)),
+	 ACPI_STRUCT_INIT (extended_function, acpi_hw_extended_sleep) },
+	{ACPI_STRUCT_INIT (legacy_function,
+			   ACPI_HW_OPTIONAL_FUNCTION (acpi_hw_legacy_wake_prep)),
+	 ACPI_STRUCT_INIT (extended_function, acpi_hw_extended_wake_prep) },
+	{ACPI_STRUCT_INIT (legacy_function,
+			   ACPI_HW_OPTIONAL_FUNCTION (acpi_hw_legacy_wake)),
+	 ACPI_STRUCT_INIT (extended_function, acpi_hw_extended_wake) }
 };
 
 /*
diff --git a/include/acpi/platform/acenv.h b/include/acpi/platform/acenv.h
index 09994b063243..912563c66948 100644
--- a/include/acpi/platform/acenv.h
+++ b/include/acpi/platform/acenv.h
@@ -382,4 +382,8 @@
 #define ACPI_INIT_FUNCTION
 #endif
 
+#ifndef ACPI_STRUCT_INIT
+#define ACPI_STRUCT_INIT(field, value) value
+#endif
+
 #endif				/* __ACENV_H__ */
diff --git a/include/acpi/platform/aclinux.h b/include/acpi/platform/aclinux.h
index a39e3f67616f..047f13865608 100644
--- a/include/acpi/platform/aclinux.h
+++ b/include/acpi/platform/aclinux.h
@@ -178,6 +178,8 @@
 #define ACPI_MSG_BIOS_ERROR     KERN_ERR "ACPI BIOS Error (bug): "
 #define ACPI_MSG_BIOS_WARNING   KERN_WARNING "ACPI BIOS Warning (bug): "
 
+#define ACPI_STRUCT_INIT(field, value)	.field = value
+
 #else				/* !__KERNEL__ */
 
 #define ACPI_USE_STANDARD_HEADERS
diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
index bb2c6789c4b7..e1526554487e 100644
--- a/scripts/gcc-plugins/randomize_layout_plugin.c
+++ b/scripts/gcc-plugins/randomize_layout_plugin.c
@@ -346,10 +346,6 @@ 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

[toc] | [prev] | [next] | [standalone]

#1652051 — Re: [PATCH v2 20/20] ACPICA: Use designated initializers

The right fix is to get rid of the idiotic dispatch table entirely,
something like the minimally tested patch below:

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

[toc] | [prev] | [next] | [standalone]

#1651712 — [PATCH v2 18/20] randstruct: Enable function pointer struct detection

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 3eac97a4c7b3..f4597dbd0631 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

[toc] | [prev] | [next] | [standalone]

#1651717 — [PATCH v2 16/20] ntfs: Use ERR_CAST() to avoid cross-structure cast

When trying to propagate an error result, the error return path attempts
to retain the error, but does this with an open cast across very different
types, which the upcoming structure layout randomization plugin flags as
being potentially dangerous in the face of randomization. This is a false
positive, but what this code actually wants to do is use ERR_CAST() to
retain the error value.

Cc: Anton Altaparmakov <anton@tuxera.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/ntfs/namei.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
index 358258364616..4690cd75d8d7 100644
--- a/fs/ntfs/namei.c
+++ b/fs/ntfs/namei.c
@@ -159,7 +159,7 @@ static struct dentry *ntfs_lookup(struct inode *dir_ino, struct dentry *dent,
 					PTR_ERR(dent_inode));
 		kfree(name);
 		/* Return the error code. */
-		return (struct dentry *)dent_inode;
+		return ERR_CAST(dent_inode);
 	}
 	/* It is guaranteed that @name is no longer allocated at this point. */
 	if (MREF_ERR(mref) == -ENOENT) {
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1651718 — [PATCH v2 17/20] ocfs2: Use ERR_CAST() to avoid cross-structure cast

When trying to propagate an error result, the error return path attempts
to retain the error, but does this with an open cast across very different
types, which the upcoming structure layout randomization plugin flags as
being potentially dangerous in the face of randomization. This is a false
positive, but what this code actually wants to do is use ERR_CAST() to
retain the error value.

Cc: Mark Fasheh <mfasheh@versity.com>
Cc: Joel Becker <jlbec@evilplan.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/ocfs2/export.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ocfs2/export.c b/fs/ocfs2/export.c
index 827fc9809bc2..9f88188060db 100644
--- a/fs/ocfs2/export.c
+++ b/fs/ocfs2/export.c
@@ -119,7 +119,7 @@ static struct dentry *ocfs2_get_dentry(struct super_block *sb,
 
 	if (IS_ERR(inode)) {
 		mlog_errno(PTR_ERR(inode));
-		result = (void *)inode;
+		result = ERR_CAST(inode);
 		goto bail;
 	}
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1651719 — [PATCH v2 19/20] [RFC] task_struct: Allow randomized layout

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. (But I'd rather find a way to avoid the #ifdef
entirely.)

One question about formatting remains: should this patch indent all the
randomized fields, due to the added unnamed struct, which would make
this patch white-space huge, or should I leave the indentation level
alone, to avoid massive churn? I opted for making the patch more readable,
but can easily do the indentation here or as a separate patch. It's not
obvious to me what is the least invasive change to make...

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/sched.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index b1298ad4da63..a9f7f957169c 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -490,6 +490,12 @@ struct task_struct {
 #endif
 	/* -1 unrunnable, 0 runnable, >0 stopped: */
 	volatile long			state;
+
+	/* This begins the randomizable portion of task_struct... */
+#if GCC_VERSION >= 40600
+	struct {
+#endif
+
 	void				*stack;
 	atomic_t			usage;
 	/* Per task flags (PF_*), defined further below: */
@@ -1052,6 +1058,14 @@ struct task_struct {
 	/* Used by LSM modules for access restriction: */
 	void				*security;
 #endif
+
+	/*
+	 * New fields for task_struct should be added above here.
+	 */
+#if GCC_VERSION >= 40600
+	} __randomize_layout;
+#endif
+
 	/* CPU-specific state of this task: */
 	struct thread_struct		thread;
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1651722 — [PATCH v2 10/20] randstruct: opt-out externally exposed function pointer structs

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

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

[toc] | [prev] | [next] | [standalone]

#1651724 — [PATCH v2 12/20] sgi-xp: Use designated initializers

Prepare to mark sensitive kernel structures for randomization by making
sure they're using designated initializers. These were identified during
allyesconfig builds of x86, arm, and arm64, with most initializer fixes
extracted from grsecurity.

To avoid casting, this implements dummy functions with full function
prototypes.

Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Robin Holt <robinmholt@gmail.com>
---
 drivers/misc/sgi-xp/xp_main.c | 59 ++++++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/drivers/misc/sgi-xp/xp_main.c b/drivers/misc/sgi-xp/xp_main.c
index 01be66d02ca8..bb47f9d9b68a 100644
--- a/drivers/misc/sgi-xp/xp_main.c
+++ b/drivers/misc/sgi-xp/xp_main.c
@@ -71,20 +71,44 @@ EXPORT_SYMBOL_GPL(xpc_registrations);
 /*
  * Initialize the XPC interface to indicate that XPC isn't loaded.
  */
-static enum xp_retval
-xpc_notloaded(void)
+static void xpc_notloaded_connect(int ch_number)
+{ }
+
+static void xpc_notloaded_disconnect(int ch_number)
+{ }
+
+static enum xp_retval xpc_notloaded_send(short partid, int ch_number,
+					 u32 flags, void *payload,
+					 u16 payload_size)
+{
+	return xpNotLoaded;
+}
+
+static enum xp_retval xpc_notloaded_send_notify(short partid, int ch_number,
+						u32 flags, void *payload,
+						u16 payload_size,
+						xpc_notify_func func,
+						void *key)
+{
+	return xpNotLoaded;
+}
+
+static void xpc_notloaded_received(short partid, int ch_number, void *payload)
+{ }
+
+static enum xp_retval xpc_notloaded_partid_to_nasids(short partid,
+						     void *nasid_mask)
 {
 	return xpNotLoaded;
 }
 
 struct xpc_interface xpc_interface = {
-	(void (*)(int))xpc_notloaded,
-	(void (*)(int))xpc_notloaded,
-	(enum xp_retval(*)(short, int, u32, void *, u16))xpc_notloaded,
-	(enum xp_retval(*)(short, int, u32, void *, u16, xpc_notify_func,
-			   void *))xpc_notloaded,
-	(void (*)(short, int, void *))xpc_notloaded,
-	(enum xp_retval(*)(short, void *))xpc_notloaded
+	.connect = xpc_notloaded_connect,
+	.disconnect = xpc_notloaded_disconnect,
+	.send = xpc_notloaded_send,
+	.send_notify = xpc_notloaded_send_notify,
+	.received = xpc_notloaded_received,
+	.partid_to_nasids = xpc_notloaded_partid_to_nasids
 };
 EXPORT_SYMBOL_GPL(xpc_interface);
 
@@ -115,17 +139,12 @@ EXPORT_SYMBOL_GPL(xpc_set_interface);
 void
 xpc_clear_interface(void)
 {
-	xpc_interface.connect = (void (*)(int))xpc_notloaded;
-	xpc_interface.disconnect = (void (*)(int))xpc_notloaded;
-	xpc_interface.send = (enum xp_retval(*)(short, int, u32, void *, u16))
-	    xpc_notloaded;
-	xpc_interface.send_notify = (enum xp_retval(*)(short, int, u32, void *,
-						       u16, xpc_notify_func,
-						       void *))xpc_notloaded;
-	xpc_interface.received = (void (*)(short, int, void *))
-	    xpc_notloaded;
-	xpc_interface.partid_to_nasids = (enum xp_retval(*)(short, void *))
-	    xpc_notloaded;
+	xpc_interface.connect = xpc_notloaded_connect;
+	xpc_interface.disconnect = xpc_notloaded_disconnect;
+	xpc_interface.send = xpc_notloaded_send;
+	xpc_interface.send_notify = xpc_notloaded_send_notify;
+	xpc_interface.received = xpc_notloaded_received;
+	xpc_interface.partid_to_nasids = xpc_notloaded_partid_to_nasids;
 }
 EXPORT_SYMBOL_GPL(xpc_clear_interface);
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1651810 — Re: [PATCH v2 12/20] sgi-xp: Use designated initializers

On Fri, May 26, 2017 at 01:17:16PM -0700, Kees Cook wrote:
> Prepare to mark sensitive kernel structures for randomization by making
> sure they're using designated initializers. These were identified during
> allyesconfig builds of x86, arm, and arm64, with most initializer fixes
> extracted from grsecurity.
> 
> To avoid casting, this implements dummy functions with full function
> prototypes.

Can we get this in ASAP?

> +static void xpc_notloaded_connect(int ch_number)
> +{ }
> +
> +static void xpc_notloaded_disconnect(int ch_number)
> +{ }

Also it might be a good idea to just check in the caller if the
methods are defined instead of having all these dummys.  A branch
is usually cheaper than an indirect call, AND it leads to less code,
AND is is less fragile.

[toc] | [prev] | [next] | [standalone]

#1651726 — [PATCH v2 01/20] NFS: Avoid cross-structure casting

When the call to nfs_devname() fails, the error path attempts to retain
the error via the mnt variable, but this requires a cast across very
different types (char * to struct vfsmount *), which the upcoming
structure layout randomization plugin flags as being potentially
dangerous in the face of randomization. This is a false positive, but
what this code actually wants to do is retain the error value, so this
patch explicitly sets it, instead of using what seems to be an
unexpected cast.

Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/namespace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index 1a224a33a6c2..e5686be67be8 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -246,7 +246,7 @@ struct vfsmount *nfs_do_submount(struct dentry *dentry, struct nfs_fh *fh,
 
 	devname = nfs_devname(dentry, page, PAGE_SIZE);
 	if (IS_ERR(devname))
-		mnt = (struct vfsmount *)devname;
+		mnt = ERR_CAST(devname);
 	else
 		mnt = nfs_do_clone_mount(NFS_SB(dentry->d_sb), devname, &mountdata);
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1652053 — Re: [PATCH v2 01/20] NFS: Avoid cross-structure casting

On Fri, May 26, 2017 at 01:17:05PM -0700, Kees Cook wrote:
> When the call to nfs_devname() fails, the error path attempts to retain
> the error via the mnt variable, but this requires a cast across very
> different types (char * to struct vfsmount *), which the upcoming
> structure layout randomization plugin flags as being potentially
> dangerous in the face of randomization. This is a false positive, but
> what this code actually wants to do is retain the error value, so this
> patch explicitly sets it, instead of using what seems to be an
> unexpected cast.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Acked-by: Trond Myklebust <trond.myklebust@primarydata.com>

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

And I think we should get this (and the other cast patches) in ASAP,
independent of the fat of the randstruct plugin.

[toc] | [prev] | [next] | [standalone]

#1652179 — Re: [PATCH v2 01/20] NFS: Avoid cross-structure casting

On Sun, May 28, 2017 at 12:53 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Fri, May 26, 2017 at 01:17:05PM -0700, Kees Cook wrote:
>> When the call to nfs_devname() fails, the error path attempts to retain
>> the error via the mnt variable, but this requires a cast across very
>> different types (char * to struct vfsmount *), which the upcoming
>> structure layout randomization plugin flags as being potentially
>> dangerous in the face of randomization. This is a false positive, but
>> what this code actually wants to do is retain the error value, so this
>> patch explicitly sets it, instead of using what seems to be an
>> unexpected cast.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> Acked-by: Trond Myklebust <trond.myklebust@primarydata.com>
>
> Looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> And I think we should get this (and the other cast patches) in ASAP,
> independent of the fat of the randstruct plugin.

Should I send a pull request to Linus for these for v4.12?

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1651727 — [PATCH v2 03/20] compiler: Add __designated_init annotation

This allows structure annotations for requiring designated initialization
in GCC 5.1.0 and later:
https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html

The structure randomization layout plugin will be using this to help
identify structures that need this form of initialization.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/compiler-gcc.h | 8 ++++++++
 include/linux/compiler.h     | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 0efef9cf014f..386caf6771ed 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -294,6 +294,14 @@
 #define __no_sanitize_address __attribute__((no_sanitize_address))
 #endif
 
+#if GCC_VERSION >= 50100
+/*
+ * Mark structures as requiring designated initializers.
+ * https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
+ */
+#define __designated_init __attribute__((designated_init))
+#endif
+
 #endif	/* gcc version >= 40000 specific checks */
 
 #if !defined(__noclone)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 707242fdbb89..bbe881054bf0 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -436,6 +436,10 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 # define __attribute_const__	/* unimplemented */
 #endif
 
+#ifndef __designated_init
+# define __designated_init
+#endif
+
 #ifndef __latent_entropy
 # define __latent_entropy
 #endif
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1651728 — [PATCH v2 08/20] randstruct: Whitelist NIU struct page overloading

The NIU ethernet driver intentionally stores a page struct pointer on
top of the "mapping" field. Whitelist this case:

drivers/net/ethernet/sun/niu.c: In function ‘niu_rx_pkt_ignore’:
drivers/net/ethernet/sun/niu.c:3402:10: note: found mismatched ssa struct pointer types: ‘struct page’ and ‘struct address_space’

    *link = (struct page *) page->mapping;
    ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 scripts/gcc-plugins/randomize_layout_plugin.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
index 5f4e59a78eeb..f777ead58ba8 100644
--- a/scripts/gcc-plugins/randomize_layout_plugin.c
+++ b/scripts/gcc-plugins/randomize_layout_plugin.c
@@ -43,6 +43,8 @@ struct whitelist_entry {
 };
 
 static const struct whitelist_entry whitelist[] = {
+	/* NIU overloads mapping with page struct */
+	{ "drivers/net/ethernet/sun/niu.c", "page", "address_space" },
 	/* unix_skb_parms via UNIXCB() buffer */
 	{ "net/unix/af_unix.c", "unix_skb_parms", "char" },
 	/* big_key payload.data struct splashing */
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1652055 — Re: [PATCH v2 08/20] randstruct: Whitelist NIU struct page overloading

On Fri, May 26, 2017 at 01:17:12PM -0700, Kees Cook wrote:
> The NIU ethernet driver intentionally stores a page struct pointer on
> top of the "mapping" field. Whitelist this case:
> 
> drivers/net/ethernet/sun/niu.c: In function ‘niu_rx_pkt_ignore’:
> drivers/net/ethernet/sun/niu.c:3402:10: note: found mismatched ssa struct pointer types: ‘struct page’ and ‘struct address_space’
> 
>     *link = (struct page *) page->mapping;
>     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Kees Cook <keescook@chromium.org>

The driver really needs to stop doing this anyway.  It would be good
to send this out to linux-mm and netdev to come up with a better scheme.

[toc] | [prev] | [next] | [standalone]

#1652184 — Re: [PATCH v2 08/20] randstruct: Whitelist NIU struct page overloading

[trying again with correct linux-mm address...]

On Sun, May 28, 2017 at 1:15 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Fri, May 26, 2017 at 01:17:12PM -0700, Kees Cook wrote:
>> The NIU ethernet driver intentionally stores a page struct pointer on
>> top of the "mapping" field. Whitelist this case:
>>
>> drivers/net/ethernet/sun/niu.c: In function ‘niu_rx_pkt_ignore’:
>> drivers/net/ethernet/sun/niu.c:3402:10: note: found mismatched ssa struct pointer types: ‘struct page’ and ‘struct address_space’
>>
>>     *link = (struct page *) page->mapping;
>>     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Cc: David S. Miller <davem@davemloft.net>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>
> The driver really needs to stop doing this anyway.  It would be good
> to send this out to linux-mm and netdev to come up with a better scheme.

Added to To. :) I couldn't understand why it was doing what it was
doing, hence the whitelist entry.

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1652185 — Re: [PATCH v2 08/20] randstruct: Whitelist NIU struct page overloading

On Sun, May 28, 2017 at 1:15 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Fri, May 26, 2017 at 01:17:12PM -0700, Kees Cook wrote:
>> The NIU ethernet driver intentionally stores a page struct pointer on
>> top of the "mapping" field. Whitelist this case:
>>
>> drivers/net/ethernet/sun/niu.c: In function ‘niu_rx_pkt_ignore’:
>> drivers/net/ethernet/sun/niu.c:3402:10: note: found mismatched ssa struct pointer types: ‘struct page’ and ‘struct address_space’
>>
>>     *link = (struct page *) page->mapping;
>>     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Cc: David S. Miller <davem@davemloft.net>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>
> The driver really needs to stop doing this anyway.  It would be good
> to send this out to linux-mm and netdev to come up with a better scheme.

Added to To. :) I couldn't understand why it was doing what it was
doing, hence the whitelist entry.

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web