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


Groups > linux.kernel > #1612221 > unrolled thread

[RFC v2] Introduce rare_write() infrastructure

Started byKees Cook <keescook@chromium.org>
First post2017-03-29 20:20 +0200
Last post2017-03-29 21:20 +0200
Articles 20 on this page of 56 — 13 participants

Back to article view | Back to linux.kernel


Contents

  [RFC v2] Introduce rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 10/11] gcc-plugins: Add constify plugin Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 03/11] net: switch sock_diag handlers to rare_write() Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 07/11] ARM: mm: set DOMAIN_WR_RARE for rodata Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
      Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-29 20:30 +0200
        Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure Ho-Eun Ryu <hoeun.ryu@gmail.com> - 2017-03-30 09:50 +0200
          Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-30 19:10 +0200
      Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure Ho-Eun Ryu <hoeun.ryu@gmail.com> - 2017-04-07 10:20 +0200
        Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-04-07 22:40 +0200
    [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
      Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@amacapital.net> - 2017-03-30 00:40 +0200
        Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Kees Cook <keescook@chromium.org> - 2017-03-30 03:50 +0200
          Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-06 02:00 +0200
            Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Kees Cook <keescook@chromium.org> - 2017-04-06 02:20 +0200
              Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@amacapital.net> - 2017-04-06 18:00 +0200
                Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Mathias Krause <minipli@googlemail.com> - 2017-04-07 10:40 +0200
                  Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Thomas Gleixner <tglx@linutronix.de> - 2017-04-07 11:50 +0200
                    Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Mathias Krause <minipli@googlemail.com> - 2017-04-07 13:00 +0200
                      Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Thomas Gleixner <tglx@linutronix.de> - 2017-04-07 15:20 +0200
                        Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Mathias Krause <minipli@googlemail.com> - 2017-04-07 15:40 +0200
                          Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-07 18:20 +0200
                            Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Mark Rutland <mark.rutland@arm.com> - 2017-04-07 18:30 +0200
                            Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() "PaX Team" <pageexec@freemail.hu> - 2017-04-07 22:10 +0200
                              Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-08 07:00 +0200
                                Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() "PaX Team" <pageexec@freemail.hu> - 2017-04-09 14:50 +0200
                                  Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-10 02:20 +0200
                                    Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() "PaX Team" <pageexec@freemail.hu> - 2017-04-10 12:50 +0200
                            Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Thomas Gleixner <tglx@linutronix.de> - 2017-04-07 22:50 +0200
                              Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Kees Cook <keescook@chromium.org> - 2017-04-07 23:30 +0200
                                Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Daniel Micay <danielmicay@gmail.com> - 2017-04-08 06:20 +0200
                                  Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Daniel Micay <danielmicay@gmail.com> - 2017-04-08 06:20 +0200
                            Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Daniel Micay <danielmicay@gmail.com> - 2017-04-08 06:30 +0200
                              Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-08 07:10 +0200
                                Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Daniel Micay <danielmicay@gmail.com> - 2017-04-08 09:40 +0200
                                  Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-08 17:30 +0200
                                    Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Ingo Molnar <mingo@kernel.org> - 2017-04-09 13:00 +0200
                                    Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Mark Rutland <mark.rutland@arm.com> - 2017-04-10 12:30 +0200
                                Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() "PaX Team" <pageexec@freemail.hu> - 2017-04-09 22:30 +0200
                                  Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-10 02:40 +0200
                          Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Thomas Gleixner <tglx@linutronix.de> - 2017-04-07 21:30 +0200
                      Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Peter Zijlstra <peterz@infradead.org> - 2017-04-07 16:50 +0200
                        Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Mark Rutland <mark.rutland@arm.com> - 2017-04-10 12:40 +0200
                    Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() "PaX Team" <pageexec@freemail.hu> - 2017-04-07 22:40 +0200
                      Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Thomas Gleixner <tglx@linutronix.de> - 2017-04-10 10:30 +0200
      Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Peter Zijlstra <peterz@infradead.org> - 2017-04-07 11:40 +0200
    [RFC v2][PATCH 09/11] list: add rare_write() list helpers Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 05/11] ARM: mm: dump: Add domain to output Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 11/11] cgroups: force all struct cftype const Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 08/11] ARM: Implement __arch_rare_write_begin/end() Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
      Re: [RFC v2][PATCH 08/11] ARM: Implement  __arch_rare_write_begin/end() Peter Zijlstra <peterz@infradead.org> - 2017-04-07 11:40 +0200
    [RFC v2][PATCH 02/11] lkdtm: add test for rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
      Re: [kernel-hardening] [RFC v2][PATCH 02/11] lkdtm: add test for  rare_write() infrastructure Ian Campbell <ijc@hellion.org.uk> - 2017-03-30 12:20 +0200
        Re: [kernel-hardening] [RFC v2][PATCH 02/11] lkdtm: add test for  rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-30 18:20 +0200
    Re: [RFC v2] Introduce rare_write() infrastructure Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-29 21:10 +0200
      Re: [RFC v2] Introduce rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-29 21:20 +0200

Page 1 of 3  [1] 2 3  Next page →


#1612221 — [RFC v2] Introduce rare_write() infrastructure

FromKees Cook <keescook@chromium.org>
Date2017-03-29 20:20 +0200
Subject[RFC v2] Introduce rare_write() infrastructure
Message-ID<tqqml-2iB-7@gated-at.bofh.it>
This is take 2 of an RFC series to demonstrate a possible infrastructure
for the "write rarely" memory storage type in the kernel (patch 1). The
intent is to further reduce the internal attack surface of the kernel
by making more variables read-only while "at rest". This is heavily
based on the "__read_only" portion of the KERNEXEC infrastructure from
PaX/grsecurity, though I tried to adjust it to be more in line with
the upstream discussions around the APIs.

Also included is the PaX/grsecurity constify plugin (patch 10) which will
automatically make all instances of certain structures read-only, to help
demonstrate more complex cases of "write rarely" targets. (The plugin in
this series is altered to only operate on marked structures, rather than
the full automatic constification.)

As part of the series I've included both x86 support (patch 4), exactly
as done in PaX, and ARM support (patches 5-7), similar to what is done in
grsecurity but without support for earlier ARM CPUs. Both are lightly
tested by me, though they need a bit more work, especially ARM as it is
missing the correct domain marking for kernel modules.

I've added an lkdtm test (patch 2), which serves as a stand-alone example
of the new infrastructure.

Included are two example "conversions" to the rare_write()-style of
variable manipulation: a simple one, which switches the inet diag handler
table to write-rarely during register/unregister calls (patch 3), and
a more complex one: cgroup types (patch 11), which is made read-only via
the constify plugin. The latter uses rare-write linked lists (patch 9)
and multi-field updates. Both examples are refactorings of what already
appears in PaX/grsecurity.

The patches are:

	[PATCH 01/11] Introduce rare_write() infrastructure
	[PATCH 02/11] lkdtm: add test for rare_write() infrastructure
	[PATCH 03/11] net: switch sock_diag handlers to rare_write()
	[PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()
	[PATCH 05/11] ARM: mm: dump: Add domain to output
	[PATCH 06/11] ARM: domains: Extract common USER domain init
	[PATCH 07/11] ARM: mm: set DOMAIN_WR_RARE for rodata
	[PATCH 08/11] ARM: Implement __arch_rare_write_begin/unmap()
	[PATCH 09/11] list: add rare_write() list helpers
	[PATCH 10/11] gcc-plugins: Add constify plugin
	[PATCH 11/11] cgroups: force all struct cftype const

-Kees

[toc] | [next] | [standalone]


#1612224 — [RFC v2][PATCH 10/11] gcc-plugins: Add constify plugin

FromKees Cook <keescook@chromium.org>
Date2017-03-29 20:20 +0200
Subject[RFC v2][PATCH 10/11] gcc-plugins: Add constify plugin
Message-ID<tqqml-2iB-13@gated-at.bofh.it>
In reply to#1612221
This is a port of the PaX/grsecurity constify plugin. However, it has
the automatic function-pointer struct detection temporarily disabled. As
a result, this will only recognize the __do_const annotation, which
makes all instances of a marked structure read-only. The rare_write()
infrastructure can be used to make changes to such variables.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/Kconfig                          |  13 +
 include/linux/compiler-gcc.h          |   5 +
 include/linux/compiler.h              |   8 +
 scripts/Makefile.gcc-plugins          |   2 +
 scripts/gcc-plugins/constify_plugin.c | 585 ++++++++++++++++++++++++++++++++++
 5 files changed, 613 insertions(+)
 create mode 100644 scripts/gcc-plugins/constify_plugin.c

diff --git a/arch/Kconfig b/arch/Kconfig
index 5ebf62500b99..79447d6a40bc 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -436,6 +436,19 @@ config GCC_PLUGIN_STRUCTLEAK_VERBOSE
 	  initialized. Since not all existing initializers are detected
 	  by the plugin, this can produce false positive warnings.
 
+config GCC_PLUGIN_CONSTIFY
+	bool "Make function pointer structures and marked variables read-only"
+	depends on GCC_PLUGINS && HAVE_ARCH_RARE_WRITE && !UML
+	help
+	  By saying Y here the compiler will automatically constify a class
+	  of types that contain only function pointers, as well as any
+	  manually annotated structures. This reduces the kernel's attack
+	  surface and also produces a better memory layout.
+
+	  This plugin was ported from grsecurity/PaX. More information at:
+	   * https://grsecurity.net/
+	   * https://pax.grsecurity.net/
+
 config HAVE_CC_STACKPROTECTOR
 	bool
 	help
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 0efef9cf014f..cee2bf7c32a4 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -191,6 +191,11 @@
 
 #if GCC_VERSION >= 40500
 
+#ifdef CONSTIFY_PLUGIN
+#define __no_const __attribute__((no_const))
+#define __do_const __attribute__((do_const))
+#endif
+
 #ifndef __CHECKER__
 #ifdef LATENT_ENTROPY_PLUGIN
 #define __latent_entropy __attribute__((latent_entropy))
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 274bd03cfe9e..2def0f12a71c 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -476,6 +476,14 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 # define __latent_entropy
 #endif
 
+#ifndef __do_const
+# define __do_const
+#endif
+
+#ifndef __no_const
+# define __no_const
+#endif
+
 /*
  * Tell gcc if a function is cold. The compiler will assume any path
  * directly leading to the call is unlikely.
diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index 82335533620e..8d264c0bb758 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -5,6 +5,8 @@ ifdef CONFIG_GCC_PLUGINS
   SANCOV_PLUGIN := -fplugin=$(objtree)/scripts/gcc-plugins/sancov_plugin.so
 
   gcc-plugin-$(CONFIG_GCC_PLUGIN_CYC_COMPLEXITY)	+= cyc_complexity_plugin.so
+  gcc-plugin-$(CONFIG_GCC_PLUGIN_CONSTIFY)	+= constify_plugin.so
+  gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_CONSTIFY)	+= -DCONSTIFY_PLUGIN
 
   gcc-plugin-$(CONFIG_GCC_PLUGIN_LATENT_ENTROPY)	+= latent_entropy_plugin.so
   gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_LATENT_ENTROPY)	+= -DLATENT_ENTROPY_PLUGIN
diff --git a/scripts/gcc-plugins/constify_plugin.c b/scripts/gcc-plugins/constify_plugin.c
new file mode 100644
index 000000000000..e7d6f3140e87
--- /dev/null
+++ b/scripts/gcc-plugins/constify_plugin.c
@@ -0,0 +1,585 @@
+/*
+ * Copyright 2011 by Emese Revfy <re.emese@gmail.com>
+ * Copyright 2011-2016 by PaX Team <pageexec@freemail.hu>
+ * Licensed under the GPL v2, or (at your option) v3
+ *
+ * This gcc plugin constifies all structures which contain only function pointers or are explicitly marked for constification.
+ *
+ * Homepage:
+ * http://www.grsecurity.net/~ephox/const_plugin/
+ *
+ * Usage:
+ * $ gcc -I`gcc -print-file-name=plugin`/include -fPIC -shared -O2 -o constify_plugin.so constify_plugin.c
+ * $ gcc -fplugin=constify_plugin.so test.c -O2
+ */
+
+#include "gcc-common.h"
+
+// unused C type flag in all versions 4.5-6
+#define TYPE_CONSTIFY_VISITED(TYPE) TYPE_LANG_FLAG_4(TYPE)
+
+__visible int plugin_is_GPL_compatible;
+
+static bool enabled = true;
+
+static struct plugin_info const_plugin_info = {
+	.version	= "201607241840vanilla",
+	.help		= "disable\tturn off constification\n",
+};
+
+static struct {
+	const char *name;
+	const char *asm_op;
+} const_sections[] = {
+	{".init.rodata",     "\t.section\t.init.rodata,\"a\""},
+	{".ref.rodata",      "\t.section\t.ref.rodata,\"a\""},
+	{".devinit.rodata",  "\t.section\t.devinit.rodata,\"a\""},
+	{".devexit.rodata",  "\t.section\t.devexit.rodata,\"a\""},
+	{".cpuinit.rodata",  "\t.section\t.cpuinit.rodata,\"a\""},
+	{".cpuexit.rodata",  "\t.section\t.cpuexit.rodata,\"a\""},
+	{".meminit.rodata",  "\t.section\t.meminit.rodata,\"a\""},
+	{".memexit.rodata",  "\t.section\t.memexit.rodata,\"a\""},
+	{".data..read_only", "\t.section\t.data..read_only,\"a\""},
+};
+
+typedef struct {
+	bool has_fptr_field;
+	bool has_writable_field;
+	bool has_do_const_field;
+	bool has_no_const_field;
+} constify_info;
+
+static const_tree get_field_type(const_tree field)
+{
+	return strip_array_types(TREE_TYPE(field));
+}
+
+static bool is_fptr(const_tree field)
+{
+	/* XXX: disable automatic constification. */
+	return false;
+
+	const_tree ptr = get_field_type(field);
+
+	if (TREE_CODE(ptr) != POINTER_TYPE)
+		return false;
+
+	return TREE_CODE(TREE_TYPE(ptr)) == FUNCTION_TYPE;
+}
+
+/*
+ * determine whether the given structure type meets the requirements for automatic constification,
+ * including the constification attributes on nested structure types
+ */
+static void constifiable(const_tree node, constify_info *cinfo)
+{
+	const_tree field;
+
+	gcc_assert(TREE_CODE(node) == RECORD_TYPE || TREE_CODE(node) == UNION_TYPE);
+
+	// e.g., pointer to structure fields while still constructing the structure type
+	if (TYPE_FIELDS(node) == NULL_TREE)
+		return;
+
+	for (field = TYPE_FIELDS(node); field; field = TREE_CHAIN(field)) {
+		const_tree type = get_field_type(field);
+		enum tree_code code = TREE_CODE(type);
+
+		if (node == type)
+			continue;
+
+		if (is_fptr(field))
+			cinfo->has_fptr_field = true;
+		else if (code == RECORD_TYPE || code == UNION_TYPE) {
+			if (lookup_attribute("do_const", TYPE_ATTRIBUTES(type)))
+				cinfo->has_do_const_field = true;
+			else if (lookup_attribute("no_const", TYPE_ATTRIBUTES(type)))
+				cinfo->has_no_const_field = true;
+			else
+				constifiable(type, cinfo);
+		} else if (!TREE_READONLY(field))
+			cinfo->has_writable_field = true;
+	}
+}
+
+static bool constified(const_tree node)
+{
+	constify_info cinfo = {
+		.has_fptr_field = false,
+		.has_writable_field = false,
+		.has_do_const_field = false,
+		.has_no_const_field = false
+	};
+
+	gcc_assert(TREE_CODE(node) == RECORD_TYPE || TREE_CODE(node) == UNION_TYPE);
+
+	if (lookup_attribute("no_const", TYPE_ATTRIBUTES(node))) {
+//		gcc_assert(!TYPE_READONLY(node));
+		return false;
+	}
+
+	if (lookup_attribute("do_const", TYPE_ATTRIBUTES(node))) {
+		gcc_assert(TYPE_READONLY(node));
+		return true;
+	}
+
+	constifiable(node, &cinfo);
+	if ((!cinfo.has_fptr_field || cinfo.has_writable_field || cinfo.has_no_const_field) && !cinfo.has_do_const_field)
+		return false;
+
+	return TYPE_READONLY(node);
+}
+
+static void deconstify_tree(tree node);
+
+static void deconstify_type(tree type)
+{
+	tree field;
+
+	gcc_assert(TREE_CODE(type) == RECORD_TYPE || TREE_CODE(type) == UNION_TYPE);
+
+	for (field = TYPE_FIELDS(type); field; field = TREE_CHAIN(field)) {
+		const_tree fieldtype = get_field_type(field);
+
+		// special case handling of simple ptr-to-same-array-type members
+		if (TREE_CODE(TREE_TYPE(field)) == POINTER_TYPE) {
+			tree ptrtype = TREE_TYPE(TREE_TYPE(field));
+
+			if (TREE_TYPE(TREE_TYPE(field)) == type)
+				continue;
+			if (TREE_CODE(ptrtype) != RECORD_TYPE && TREE_CODE(ptrtype) != UNION_TYPE)
+				continue;
+			if (!constified(ptrtype))
+				continue;
+			if (TYPE_MAIN_VARIANT(ptrtype) == TYPE_MAIN_VARIANT(type))
+				TREE_TYPE(field) = build_pointer_type(build_qualified_type(type, TYPE_QUALS(ptrtype) & ~TYPE_QUAL_CONST));
+			continue;
+		}
+		if (TREE_CODE(fieldtype) != RECORD_TYPE && TREE_CODE(fieldtype) != UNION_TYPE)
+			continue;
+		if (!constified(fieldtype))
+			continue;
+
+		deconstify_tree(field);
+		TREE_READONLY(field) = 0;
+	}
+	TYPE_READONLY(type) = 0;
+	C_TYPE_FIELDS_READONLY(type) = 0;
+	if (lookup_attribute("do_const", TYPE_ATTRIBUTES(type))) {
+		TYPE_ATTRIBUTES(type) = copy_list(TYPE_ATTRIBUTES(type));
+		TYPE_ATTRIBUTES(type) = remove_attribute("do_const", TYPE_ATTRIBUTES(type));
+	}
+}
+
+static void deconstify_tree(tree node)
+{
+	tree old_type, new_type, field;
+
+	old_type = TREE_TYPE(node);
+	while (TREE_CODE(old_type) == ARRAY_TYPE && TREE_CODE(TREE_TYPE(old_type)) != ARRAY_TYPE) {
+		node = TREE_TYPE(node) = copy_node(old_type);
+		old_type = TREE_TYPE(old_type);
+	}
+
+	gcc_assert(TREE_CODE(old_type) == RECORD_TYPE || TREE_CODE(old_type) == UNION_TYPE);
+	gcc_assert(TYPE_READONLY(old_type) && (TYPE_QUALS(old_type) & TYPE_QUAL_CONST));
+
+	new_type = build_qualified_type(old_type, TYPE_QUALS(old_type) & ~TYPE_QUAL_CONST);
+	TYPE_FIELDS(new_type) = copy_list(TYPE_FIELDS(new_type));
+	for (field = TYPE_FIELDS(new_type); field; field = TREE_CHAIN(field))
+		DECL_FIELD_CONTEXT(field) = new_type;
+
+	deconstify_type(new_type);
+
+	TREE_TYPE(node) = new_type;
+}
+
+static tree handle_no_const_attribute(tree *node, tree name, tree args, int flags, bool *no_add_attrs)
+{
+	tree type;
+	constify_info cinfo = {
+		.has_fptr_field = false,
+		.has_writable_field = false,
+		.has_do_const_field = false,
+		.has_no_const_field = false
+	};
+
+	*no_add_attrs = true;
+	if (TREE_CODE(*node) == FUNCTION_DECL) {
+		error("%qE attribute does not apply to functions (%qF)", name, *node);
+		return NULL_TREE;
+	}
+
+	if (TREE_CODE(*node) == PARM_DECL) {
+		error("%qE attribute does not apply to function parameters (%qD)", name, *node);
+		return NULL_TREE;
+	}
+
+	if (TREE_CODE(*node) == VAR_DECL) {
+		error("%qE attribute does not apply to variables (%qD)", name, *node);
+		return NULL_TREE;
+	}
+
+	if (TYPE_P(*node)) {
+		type = *node;
+	} else {
+		if (TREE_CODE(*node) != TYPE_DECL) {
+			error("%qE attribute does not apply to %qD (%qT)", name, *node, TREE_TYPE(*node));
+			return NULL_TREE;
+		}
+		type = TREE_TYPE(*node);
+	}
+
+	if (TREE_CODE(type) != RECORD_TYPE && TREE_CODE(type) != UNION_TYPE) {
+		error("%qE attribute used on %qT applies to struct and union types only", name, type);
+		return NULL_TREE;
+	}
+
+	if (lookup_attribute(IDENTIFIER_POINTER(name), TYPE_ATTRIBUTES(type))) {
+		error("%qE attribute is already applied to the type %qT", name, type);
+		return NULL_TREE;
+	}
+
+	if (TYPE_P(*node)) {
+		if (lookup_attribute("do_const", TYPE_ATTRIBUTES(type)))
+			error("%qE attribute used on type %qT is incompatible with 'do_const'", name, type);
+		else
+			*no_add_attrs = false;
+		return NULL_TREE;
+	}
+
+	constifiable(type, &cinfo);
+	if ((cinfo.has_fptr_field && !cinfo.has_writable_field && !cinfo.has_no_const_field) || lookup_attribute("do_const", TYPE_ATTRIBUTES(type))) {
+		if (enabled) {
+			if TYPE_P(*node)
+				deconstify_type(*node);
+			else
+				deconstify_tree(*node);
+		}
+		if (TYPE_P(*node))
+			TYPE_CONSTIFY_VISITED(*node) = 1;
+		else
+			TYPE_CONSTIFY_VISITED(TREE_TYPE(*node)) = 1;
+		return NULL_TREE;
+	}
+
+	if (enabled && TYPE_FIELDS(type))
+		error("%qE attribute used on type %qT that is not constified", name, type);
+	return NULL_TREE;
+}
+
+static void constify_type(tree type)
+{
+	gcc_assert(type == TYPE_MAIN_VARIANT(type));
+	TYPE_READONLY(type) = 1;
+	C_TYPE_FIELDS_READONLY(type) = 1;
+	TYPE_CONSTIFY_VISITED(type) = 1;
+//	TYPE_ATTRIBUTES(type) = copy_list(TYPE_ATTRIBUTES(type));
+//	TYPE_ATTRIBUTES(type) = tree_cons(get_identifier("do_const"), NULL_TREE, TYPE_ATTRIBUTES(type));
+}
+
+static tree handle_do_const_attribute(tree *node, tree name, tree args, int flags, bool *no_add_attrs)
+{
+	*no_add_attrs = true;
+	if (!TYPE_P(*node)) {
+		error("%qE attribute applies to types only (%qD)", name, *node);
+		return NULL_TREE;
+	}
+
+	if (TREE_CODE(*node) != RECORD_TYPE && TREE_CODE(*node) != UNION_TYPE) {
+		error("%qE attribute used on %qT applies to struct and union types only", name, *node);
+		return NULL_TREE;
+	}
+
+	if (lookup_attribute(IDENTIFIER_POINTER(name), TYPE_ATTRIBUTES(*node))) {
+		error("%qE attribute used on %qT is already applied to the type", name, *node);
+		return NULL_TREE;
+	}
+
+	if (lookup_attribute("no_const", TYPE_ATTRIBUTES(*node))) {
+		error("%qE attribute used on %qT is incompatible with 'no_const'", name, *node);
+		return NULL_TREE;
+	}
+
+	*no_add_attrs = false;
+	return NULL_TREE;
+}
+
+static struct attribute_spec no_const_attr = {
+	.name			= "no_const",
+	.min_length		= 0,
+	.max_length		= 0,
+	.decl_required		= false,
+	.type_required		= false,
+	.function_type_required	= false,
+	.handler		= handle_no_const_attribute,
+#if BUILDING_GCC_VERSION >= 4007
+	.affects_type_identity	= true
+#endif
+};
+
+static struct attribute_spec do_const_attr = {
+	.name			= "do_const",
+	.min_length		= 0,
+	.max_length		= 0,
+	.decl_required		= false,
+	.type_required		= false,
+	.function_type_required	= false,
+	.handler		= handle_do_const_attribute,
+#if BUILDING_GCC_VERSION >= 4007
+	.affects_type_identity	= true
+#endif
+};
+
+static void register_attributes(void *event_data, void *data)
+{
+	register_attribute(&no_const_attr);
+	register_attribute(&do_const_attr);
+}
+
+static void finish_type(void *event_data, void *data)
+{
+	tree type = (tree)event_data;
+	constify_info cinfo = {
+		.has_fptr_field = false,
+		.has_writable_field = false,
+		.has_do_const_field = false,
+		.has_no_const_field = false
+	};
+
+	if (type == NULL_TREE || type == error_mark_node)
+		return;
+
+#if BUILDING_GCC_VERSION >= 5000
+	if (TREE_CODE(type) == ENUMERAL_TYPE)
+		return;
+#endif
+
+	if (TYPE_FIELDS(type) == NULL_TREE || TYPE_CONSTIFY_VISITED(type))
+		return;
+
+	constifiable(type, &cinfo);
+
+	if (lookup_attribute("no_const", TYPE_ATTRIBUTES(type))) {
+		if ((cinfo.has_fptr_field && !cinfo.has_writable_field && !cinfo.has_no_const_field) || cinfo.has_do_const_field) {
+			deconstify_type(type);
+			TYPE_CONSTIFY_VISITED(type) = 1;
+		} else
+			error("'no_const' attribute used on type %qT that is not constified", type);
+		return;
+	}
+
+	if (lookup_attribute("do_const", TYPE_ATTRIBUTES(type))) {
+		if (!cinfo.has_writable_field && !cinfo.has_no_const_field) {
+			error("'do_const' attribute used on type %qT that is%sconstified", type, cinfo.has_fptr_field ? " " : " not ");
+			return;
+		}
+		constify_type(type);
+		return;
+	}
+
+	if (cinfo.has_fptr_field && !cinfo.has_writable_field && !cinfo.has_no_const_field) {
+		if (lookup_attribute("do_const", TYPE_ATTRIBUTES(type))) {
+			error("'do_const' attribute used on type %qT that is constified", type);
+			return;
+		}
+		constify_type(type);
+		return;
+	}
+
+	deconstify_type(type);
+	TYPE_CONSTIFY_VISITED(type) = 1;
+}
+
+static bool is_constified_var(varpool_node_ptr node)
+{
+	tree var = NODE_DECL(node);
+	tree type = TREE_TYPE(var);
+
+	if (node->alias)
+		return false;
+
+	if (DECL_EXTERNAL(var))
+		return false;
+
+	// XXX handle more complex nesting of arrays/structs
+	if (TREE_CODE(type) == ARRAY_TYPE)
+		type = TREE_TYPE(type);
+
+	if (TREE_CODE(type) != RECORD_TYPE && TREE_CODE(type) != UNION_TYPE)
+		return false;
+
+	if (!TYPE_READONLY(type) || !C_TYPE_FIELDS_READONLY(type))
+		return false;
+
+	if (!TYPE_CONSTIFY_VISITED(type))
+		return false;
+
+	return true;
+}
+
+static void check_section_mismatch(varpool_node_ptr node)
+{
+	tree var, section;
+	size_t i;
+	const char *name;
+
+	var = NODE_DECL(node);
+	name = get_decl_section_name(var);
+	section = lookup_attribute("section", DECL_ATTRIBUTES(var));
+	if (!section) {
+		if (name) {
+			fprintf(stderr, "DECL_SECTION [%s] ", name);
+			dump_varpool_node(stderr, node);
+			gcc_unreachable();
+		}
+		return;
+	} else
+		gcc_assert(name);
+
+//fprintf(stderr, "SECTIONAME: [%s] ", get_decl_section_name(var));
+//debug_tree(var);
+
+	gcc_assert(!TREE_CHAIN(section));
+	gcc_assert(TREE_VALUE(section));
+
+	section = TREE_VALUE(TREE_VALUE(section));
+	gcc_assert(!strcmp(TREE_STRING_POINTER(section), name));
+//debug_tree(section);
+
+	for (i = 0; i < ARRAY_SIZE(const_sections); i++)
+		if (!strcmp(const_sections[i].name, name))
+			return;
+
+	error_at(DECL_SOURCE_LOCATION(var), "constified variable %qD placed into writable section %E", var, section);
+}
+
+// this works around a gcc bug/feature where uninitialized globals
+// are moved into the .bss section regardless of any constification
+// see gcc/varasm.c:bss_initializer_p()
+static void fix_initializer(varpool_node_ptr node)
+{
+	tree var = NODE_DECL(node);
+	tree type = TREE_TYPE(var);
+
+	if (DECL_INITIAL(var))
+		return;
+
+	DECL_INITIAL(var) = build_constructor(type, NULL);
+//	inform(DECL_SOURCE_LOCATION(var), "constified variable %qE moved into .rodata", var);
+}
+
+static void check_global_variables(void *event_data, void *data)
+{
+	varpool_node_ptr node;
+
+	FOR_EACH_VARIABLE(node) {
+		if (!is_constified_var(node))
+			continue;
+
+		check_section_mismatch(node);
+		fix_initializer(node);
+	}
+}
+
+static unsigned int check_local_variables_execute(void)
+{
+	unsigned int ret = 0;
+	tree var;
+
+	unsigned int i;
+
+	FOR_EACH_LOCAL_DECL(cfun, i, var) {
+		tree type = TREE_TYPE(var);
+
+		gcc_assert(DECL_P(var));
+		if (is_global_var(var))
+			continue;
+
+		if (TREE_CODE(type) != RECORD_TYPE && TREE_CODE(type) != UNION_TYPE)
+			continue;
+
+		if (!TYPE_READONLY(type) || !C_TYPE_FIELDS_READONLY(type))
+			continue;
+
+		if (!TYPE_CONSTIFY_VISITED(type))
+			continue;
+
+		error_at(DECL_SOURCE_LOCATION(var), "constified variable %qE cannot be local", var);
+		ret = 1;
+	}
+	return ret;
+}
+
+#define PASS_NAME check_local_variables
+#define NO_GATE
+#include "gcc-generate-gimple-pass.h"
+
+static unsigned int (*old_section_type_flags)(tree decl, const char *name, int reloc);
+
+static unsigned int constify_section_type_flags(tree decl, const char *name, int reloc)
+{
+	size_t i;
+
+	for (i = 0; i < ARRAY_SIZE(const_sections); i++)
+		if (!strcmp(const_sections[i].name, name))
+			return 0;
+
+	return old_section_type_flags(decl, name, reloc);
+}
+
+static void constify_start_unit(void *gcc_data, void *user_data)
+{
+//	size_t i;
+
+//	for (i = 0; i < ARRAY_SIZE(const_sections); i++)
+//		const_sections[i].section = get_unnamed_section(0, output_section_asm_op, const_sections[i].asm_op);
+//		const_sections[i].section = get_section(const_sections[i].name, 0, NULL);
+
+	old_section_type_flags = targetm.section_type_flags;
+	targetm.section_type_flags = constify_section_type_flags;
+}
+
+__visible int plugin_init(struct plugin_name_args *plugin_info, struct plugin_gcc_version *version)
+{
+	const char * const plugin_name = plugin_info->base_name;
+	const int argc = plugin_info->argc;
+	const struct plugin_argument * const argv = plugin_info->argv;
+	int i;
+
+	struct register_pass_info check_local_variables_pass_info;
+
+	check_local_variables_pass_info.pass				= make_check_local_variables_pass();
+	check_local_variables_pass_info.reference_pass_name		= "ssa";
+	check_local_variables_pass_info.ref_pass_instance_number	= 1;
+	check_local_variables_pass_info.pos_op				= PASS_POS_INSERT_BEFORE;
+
+	if (!plugin_default_version_check(version, &gcc_version)) {
+		error(G_("incompatible gcc/plugin versions"));
+		return 1;
+	}
+
+	for (i = 0; i < argc; ++i) {
+		if (!(strcmp(argv[i].key, "disable"))) {
+			enabled = false;
+			continue;
+		}
+		error(G_("unknown option '-fplugin-arg-%s-%s'"), plugin_name, argv[i].key);
+	}
+
+	if (strncmp(lang_hooks.name, "GNU C", 5) && !strncmp(lang_hooks.name, "GNU C+", 6)) {
+		inform(UNKNOWN_LOCATION, G_("%s supports C only, not %s"), plugin_name, lang_hooks.name);
+		enabled = false;
+	}
+
+	register_callback(plugin_name, PLUGIN_INFO, NULL, &const_plugin_info);
+	if (enabled) {
+		register_callback(plugin_name, PLUGIN_ALL_IPA_PASSES_START, check_global_variables, NULL);
+		register_callback(plugin_name, PLUGIN_FINISH_TYPE, finish_type, NULL);
+		register_callback(plugin_name, PLUGIN_PASS_MANAGER_SETUP, NULL, &check_local_variables_pass_info);
+		register_callback(plugin_name, PLUGIN_START_UNIT, constify_start_unit, NULL);
+	}
+	register_callback(plugin_name, PLUGIN_ATTRIBUTES, register_attributes, NULL);
+
+	return 0;
+}
-- 
2.7.4

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


#1612225 — [RFC v2][PATCH 03/11] net: switch sock_diag handlers to rare_write()

FromKees Cook <keescook@chromium.org>
Date2017-03-29 20:20 +0200
Subject[RFC v2][PATCH 03/11] net: switch sock_diag handlers to rare_write()
Message-ID<tqqml-2iB-17@gated-at.bofh.it>
In reply to#1612221
This is a simple example a register/unregister case for __wr_rare markings,
which only needs a simple rare_write() call to make updates.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 net/core/sock_diag.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/core/sock_diag.c b/net/core/sock_diag.c
index 6b10573cc9fa..67253026106f 100644
--- a/net/core/sock_diag.c
+++ b/net/core/sock_diag.c
@@ -14,7 +14,7 @@
 #include <linux/inet_diag.h>
 #include <linux/sock_diag.h>
 
-static const struct sock_diag_handler *sock_diag_handlers[AF_MAX];
+static const struct sock_diag_handler *sock_diag_handlers[AF_MAX] __wr_rare;
 static int (*inet_rcv_compat)(struct sk_buff *skb, struct nlmsghdr *nlh);
 static DEFINE_MUTEX(sock_diag_table_mutex);
 static struct workqueue_struct *broadcast_wq;
@@ -194,7 +194,7 @@ int sock_diag_register(const struct sock_diag_handler *hndl)
 	if (sock_diag_handlers[hndl->family])
 		err = -EBUSY;
 	else
-		sock_diag_handlers[hndl->family] = hndl;
+		rare_write(sock_diag_handlers[hndl->family], hndl);
 	mutex_unlock(&sock_diag_table_mutex);
 
 	return err;
@@ -210,7 +210,7 @@ void sock_diag_unregister(const struct sock_diag_handler *hnld)
 
 	mutex_lock(&sock_diag_table_mutex);
 	BUG_ON(sock_diag_handlers[family] != hnld);
-	sock_diag_handlers[family] = NULL;
+	rare_write(sock_diag_handlers[family], NULL);
 	mutex_unlock(&sock_diag_table_mutex);
 }
 EXPORT_SYMBOL_GPL(sock_diag_unregister);
-- 
2.7.4

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


#1612228 — [RFC v2][PATCH 07/11] ARM: mm: set DOMAIN_WR_RARE for rodata

FromKees Cook <keescook@chromium.org>
Date2017-03-29 20:20 +0200
Subject[RFC v2][PATCH 07/11] ARM: mm: set DOMAIN_WR_RARE for rodata
Message-ID<tqqmm-2iB-29@gated-at.bofh.it>
In reply to#1612221
This creates DOMAIN_WR_RARE for the kernel's .rodata section, separate
from DOMAIN_KERNEL to avoid predictive fetching in device memory during
a DOMAIN_MANAGER transition.

TODO: handle kernel module vmalloc memory, which needs to be marked as
DOMAIN_WR_RARE too, for module .rodata sections.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/arm/include/asm/domain.h | 3 +++
 arch/arm/mm/dump.c            | 2 ++
 arch/arm/mm/init.c            | 7 ++++---
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/domain.h b/arch/arm/include/asm/domain.h
index 8b33bd7f6bf9..b5ca80ac823c 100644
--- a/arch/arm/include/asm/domain.h
+++ b/arch/arm/include/asm/domain.h
@@ -43,6 +43,7 @@
 #define DOMAIN_IO	0
 #endif
 #define DOMAIN_VECTORS	3
+#define DOMAIN_WR_RARE	4
 
 /*
  * Domain types
@@ -69,11 +70,13 @@
 #define DACR_INIT \
 	(__DACR_INIT_USER | \
 	 domain_val(DOMAIN_KERNEL, DOMAIN_MANAGER) | \
+	 domain_val(DOMAIN_WR_RARE, DOMAIN_CLIENT) | \
 	 domain_val(DOMAIN_IO, DOMAIN_CLIENT) | \
 	 domain_val(DOMAIN_VECTORS, DOMAIN_CLIENT))
 
 #define __DACR_DEFAULT \
 	domain_val(DOMAIN_KERNEL, DOMAIN_CLIENT) | \
+	domain_val(DOMAIN_WR_RARE, DOMAIN_CLIENT) | \
 	domain_val(DOMAIN_IO, DOMAIN_CLIENT) | \
 	domain_val(DOMAIN_VECTORS, DOMAIN_CLIENT)
 
diff --git a/arch/arm/mm/dump.c b/arch/arm/mm/dump.c
index 35ff45470dbf..b1aa9a17e0c3 100644
--- a/arch/arm/mm/dump.c
+++ b/arch/arm/mm/dump.c
@@ -288,6 +288,8 @@ static const char *get_domain_name(pmd_t *pmd)
 		return "IO     ";
 	case PMD_DOMAIN(DOMAIN_VECTORS):
 		return "VECTORS";
+	case PMD_DOMAIN(DOMAIN_WR_RARE):
+		return "WR_RARE";
 	default:
 		return "unknown";
 	}
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 1d8558ff9827..d54a74b5718b 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -642,9 +642,10 @@ static struct section_perm ro_perms[] = {
 		.mask   = ~L_PMD_SECT_RDONLY,
 		.prot   = L_PMD_SECT_RDONLY,
 #else
-		.mask   = ~(PMD_SECT_APX | PMD_SECT_AP_WRITE),
-		.prot   = PMD_SECT_APX | PMD_SECT_AP_WRITE,
-		.clear  = PMD_SECT_AP_WRITE,
+		.mask   = ~(PMD_SECT_APX | PMD_SECT_AP_WRITE | PMD_DOMAIN_MASK),
+		.prot   = PMD_SECT_APX | PMD_SECT_AP_WRITE | \
+			  PMD_DOMAIN(DOMAIN_WR_RARE),
+		.clear  = PMD_SECT_AP_WRITE | PMD_DOMAIN(DOMAIN_KERNEL),
 #endif
 	},
 };
-- 
2.7.4

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


#1612229 — [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure

FromKees Cook <keescook@chromium.org>
Date2017-03-29 20:20 +0200
Subject[RFC v2][PATCH 01/11] Introduce rare_write() infrastructure
Message-ID<tqqmm-2iB-37@gated-at.bofh.it>
In reply to#1612221
Several types of data storage exist in the kernel: read-write data (.data,
.bss), read-only data (.rodata), and RO-after-init. This introduces the
infrastructure for another type: write-rarely, which is intended for data
that is either only rarely modified or especially security-sensitive. The
goal is to further reduce the internal attack surface of the kernel by
making this storage read-only when "at rest". This makes it much harder
to be subverted by attackers who have a kernel-write flaw, since they
cannot directly change these memory contents.

This work is heavily based on PaX and grsecurity's pax_{open,close}_kernel
API, its __read_only annotations, its constify plugin, and the work done
to identify sensitive structures that should be moved from .data into
.rodata. This builds the initial infrastructure to support these kinds
of changes, though the API and naming has been adjusted in places for
clarity and maintainability.

Variables declared with the __wr_rare annotation will be moved to the
.rodata section if an architecture supports CONFIG_HAVE_ARCH_WRITE_RARE.
To change these variables, either a single rare_write() macro can be used,
or multiple uses of __rare_write(), wrapped in a matching pair of
rare_write_begin() and rare_write_end() macros can be used. These macros
are expanded into the arch-specific functions that perform the actions
needed to write to otherwise read-only memory.

As detailed in the Kconfig help, the arch-specific helpers have several
requirements to make them sensible/safe for use by the kernel: they must
not allow non-current CPUs to write the memory area, they must run
non-preemptible to avoid accidentally leaving memory writable, and must
be inline to avoid making them desirable ROP targets for attackers.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/Kconfig             | 25 +++++++++++++++++++++++++
 include/linux/compiler.h | 32 ++++++++++++++++++++++++++++++++
 include/linux/preempt.h  |  6 ++++--
 3 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index cd211a14a88f..5ebf62500b99 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -847,4 +847,29 @@ config STRICT_MODULE_RWX
 config ARCH_WANT_RELAX_ORDER
 	bool
 
+config HAVE_ARCH_RARE_WRITE
+	def_bool n
+	help
+	  An arch should select this option if it has defined the functions
+	  __arch_rare_write_begin() and __arch_rare_write_end() to
+	  respectively enable and disable writing to read-only memory. The
+	  routines must meet the following requirements:
+	  - read-only memory writing must only be available on the current
+	    CPU (to make sure other CPUs can't race to make changes too).
+	  - the routines must be declared inline (to discourage ROP use).
+	  - the routines must not be preemptible (likely they will call
+	    preempt_disable() and preempt_enable_no_resched() respectively).
+	  - the routines must validate expected state (e.g. when enabling
+	    writes, BUG() if writes are already be enabled).
+
+config HAVE_ARCH_RARE_WRITE_MEMCPY
+	def_bool n
+	depends on HAVE_ARCH_RARE_WRITE
+	help
+	  An arch should select this option if a special accessor is needed
+	  to write to otherwise read-only memory, defined by the function
+	  __arch_rare_write_memcpy(). Without this, the write-rarely
+	  infrastructure will just attempt to write directly to the memory
+	  using a const-ignoring assignment.
+
 source "kernel/gcov/Kconfig"
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index f8110051188f..274bd03cfe9e 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -336,6 +336,38 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 	__u.__val;					\
 })
 
+/*
+ * Build "write rarely" infrastructure for flipping memory r/w
+ * on a per-CPU basis.
+ */
+#ifndef CONFIG_HAVE_ARCH_RARE_WRITE
+# define __wr_rare
+# define __wr_rare_type
+# define __rare_write(__var, __val)	(__var = (__val))
+# define rare_write_begin()		do { } while (0)
+# define rare_write_end()		do { } while (0)
+#else
+# define __wr_rare			__ro_after_init
+# define __wr_rare_type			const
+# ifdef CONFIG_HAVE_ARCH_RARE_WRITE_MEMCPY
+#  define __rare_write_n(dst, src, len)	({			\
+		BUILD_BUG(!builtin_const(len));			\
+		__arch_rare_write_memcpy((dst), (src), (len));	\
+	})
+#  define __rare_write(var, val)  __rare_write_n(&(var), &(val), sizeof(var))
+# else
+#  define __rare_write(var, val)  ((*(typeof((typeof(var))0) *)&(var)) = (val))
+# endif
+# define rare_write_begin()	__arch_rare_write_begin()
+# define rare_write_end()	__arch_rare_write_end()
+#endif
+#define rare_write(__var, __val) ({			\
+	rare_write_begin();				\
+	__rare_write(__var, __val);			\
+	rare_write_end();				\
+	__var;						\
+})
+
 #endif /* __KERNEL__ */
 
 #endif /* __ASSEMBLY__ */
diff --git a/include/linux/preempt.h b/include/linux/preempt.h
index cae461224948..4fc97aaa22ea 100644
--- a/include/linux/preempt.h
+++ b/include/linux/preempt.h
@@ -258,10 +258,12 @@ do { \
 /*
  * Modules have no business playing preemption tricks.
  */
-#undef sched_preempt_enable_no_resched
-#undef preempt_enable_no_resched
 #undef preempt_enable_no_resched_notrace
 #undef preempt_check_resched
+#ifndef CONFIG_HAVE_ARCH_RARE_WRITE
+#undef sched_preempt_enable_no_resched
+#undef preempt_enable_no_resched
+#endif
 #endif
 
 #define preempt_set_need_resched() \
-- 
2.7.4

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


#1612237 — Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure

FromKees Cook <keescook@chromium.org>
Date2017-03-29 20:30 +0200
SubjectRe: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure
Message-ID<tqqw1-2lW-5@gated-at.bofh.it>
In reply to#1612229
On Wed, Mar 29, 2017 at 11:15 AM, Kees Cook <keescook@chromium.org> wrote:
> +/*
> + * Build "write rarely" infrastructure for flipping memory r/w
> + * on a per-CPU basis.
> + */
> +#ifndef CONFIG_HAVE_ARCH_RARE_WRITE
> +# define __wr_rare
> +# define __wr_rare_type
> +# define __rare_write(__var, __val)    (__var = (__val))
> +# define rare_write_begin()            do { } while (0)
> +# define rare_write_end()              do { } while (0)
> +#else
> +# define __wr_rare                     __ro_after_init
> +# define __wr_rare_type                        const
> +# ifdef CONFIG_HAVE_ARCH_RARE_WRITE_MEMCPY
> +#  define __rare_write_n(dst, src, len)        ({                      \
> +               BUILD_BUG(!builtin_const(len));                 \
> +               __arch_rare_write_memcpy((dst), (src), (len));  \
> +       })
> +#  define __rare_write(var, val)  __rare_write_n(&(var), &(val), sizeof(var))
> +# else
> +#  define __rare_write(var, val)  ((*(typeof((typeof(var))0) *)&(var)) = (val))
> +# endif
> +# define rare_write_begin()    __arch_rare_write_begin()
> +# define rare_write_end()      __arch_rare_write_end()
> +#endif
> +#define rare_write(__var, __val) ({                    \
> +       rare_write_begin();                             \
> +       __rare_write(__var, __val);                     \
> +       rare_write_end();                               \
> +       __var;                                          \
> +})
> +

Of course, only after sending this do I realize that the MEMCPY case
will need to be further adjusted, since it currently can't take
literals. I guess something like this needs to be done:

#define __rare_write(var, val) ({ \
    typeof(var) __src = (val);     \
    __rare_write_n(&(var), &(__src), sizeof(var)); \
})

-Kees

-- 
Kees Cook
Pixel Security

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


#1612681 — Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure

FromHo-Eun Ryu <hoeun.ryu@gmail.com>
Date2017-03-30 09:50 +0200
SubjectRe: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure
Message-ID<tqD0f-2Ug-41@gated-at.bofh.it>
In reply to#1612237
> On 30 Mar 2017, at 3:23 AM, Kees Cook <keescook@chromium.org> wrote:
> 
> On Wed, Mar 29, 2017 at 11:15 AM, Kees Cook <keescook@chromium.org> wrote:
>> +/*
>> + * Build "write rarely" infrastructure for flipping memory r/w
>> + * on a per-CPU basis.
>> + */
>> +#ifndef CONFIG_HAVE_ARCH_RARE_WRITE
>> +# define __wr_rare
>> +# define __wr_rare_type
>> +# define __rare_write(__var, __val)    (__var = (__val))
>> +# define rare_write_begin()            do { } while (0)
>> +# define rare_write_end()              do { } while (0)
>> +#else
>> +# define __wr_rare                     __ro_after_init
>> +# define __wr_rare_type                        const
>> +# ifdef CONFIG_HAVE_ARCH_RARE_WRITE_MEMCPY
>> +#  define __rare_write_n(dst, src, len)        ({                      \
>> +               BUILD_BUG(!builtin_const(len));                 \
>> +               __arch_rare_write_memcpy((dst), (src), (len));  \
>> +       })
>> +#  define __rare_write(var, val)  __rare_write_n(&(var), &(val), sizeof(var))
>> +# else
>> +#  define __rare_write(var, val)  ((*(typeof((typeof(var))0) *)&(var)) = (val))
>> +# endif
>> +# define rare_write_begin()    __arch_rare_write_begin()
>> +# define rare_write_end()      __arch_rare_write_end()
>> +#endif
>> +#define rare_write(__var, __val) ({                    \
>> +       rare_write_begin();                             \
>> +       __rare_write(__var, __val);                     \
>> +       rare_write_end();                               \
>> +       __var;                                          \
>> +})
>> +
> 
> Of course, only after sending this do I realize that the MEMCPY case
> will need to be further adjusted, since it currently can't take
> literals. I guess something like this needs to be done:
> 
> #define __rare_write(var, val) ({ \
>    typeof(var) __src = (val);     \
>    __rare_write_n(&(var), &(__src), sizeof(var)); \
> })
> 

Right, and it has a problem with BUILD_BUG, which causes compilation error when CONFIG_HABE_ARCH_RARE_WRITE_MEMCPY is true
BUILD_BUG is defined in <linux/bug.h> but <linux/bug.h> includes <linux/compiler.h>

Please see the following.

diff --git a/include/linux/compiler.h b/include/linux/compiler.h                
index 3334fa9..3fa50e1 100644                                                   
--- a/include/linux/compiler.h                                                  
+++ b/include/linux/compiler.h                                                  
@@ -350,11 +350,11 @@ static __always_inline void __write_once_size(volatile vo\
id *p, void *res, int s
 # define __wr_rare                     __ro_after_init                         
 # define __wr_rare_type                        const                           
 # ifdef CONFIG_HAVE_ARCH_RARE_WRITE_MEMCPY                                     
-#  define __rare_write_n(dst, src, len)        ({                      \       
-               BUILD_BUG(!builtin_const(len));                         \
-               __arch_rare_write_memcpy((dst), (src), (len));   \               
+#  define __rare_write_n(var, val, len)        ({                                 \                                                                              
+               typeof(val) __val = val;                                                    \
+               __arch_rare_write_memcpy(&(var), &(__val), (len));      \       
        })
-#  define __rare_write(var, val)  __rare_write_n(&(var), &(val), sizeof(var))  
+#  define __rare_write(var, val)  __rare_write_n((var), (val), sizeof(var))    
 # else                                                                         
 #  define __rare_write(var, val)  ((*(typeof((typeof(var))0) *)&(var)) = (val)\
)                                                                               
 # endif                                                                        


> -Kees
> 
> -- 
> Kees Cook
> Pixel Security

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


#1613299 — Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure

FromKees Cook <keescook@chromium.org>
Date2017-03-30 19:10 +0200
SubjectRe: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure
Message-ID<tqLK9-16I-1@gated-at.bofh.it>
In reply to#1612681
On Thu, Mar 30, 2017 at 12:44 AM, Ho-Eun Ryu <hoeun.ryu@gmail.com> wrote:
>
>> On 30 Mar 2017, at 3:23 AM, Kees Cook <keescook@chromium.org> wrote:
>>
>> On Wed, Mar 29, 2017 at 11:15 AM, Kees Cook <keescook@chromium.org> wrote:
>>> +/*
>>> + * Build "write rarely" infrastructure for flipping memory r/w
>>> + * on a per-CPU basis.
>>> + */
>>> +#ifndef CONFIG_HAVE_ARCH_RARE_WRITE
>>> +# define __wr_rare
>>> +# define __wr_rare_type
>>> +# define __rare_write(__var, __val)    (__var = (__val))
>>> +# define rare_write_begin()            do { } while (0)
>>> +# define rare_write_end()              do { } while (0)
>>> +#else
>>> +# define __wr_rare                     __ro_after_init
>>> +# define __wr_rare_type                        const
>>> +# ifdef CONFIG_HAVE_ARCH_RARE_WRITE_MEMCPY
>>> +#  define __rare_write_n(dst, src, len)        ({                      \
>>> +               BUILD_BUG(!builtin_const(len));                 \
>>> +               __arch_rare_write_memcpy((dst), (src), (len));  \
>>> +       })
>>> +#  define __rare_write(var, val)  __rare_write_n(&(var), &(val), sizeof(var))
>>> +# else
>>> +#  define __rare_write(var, val)  ((*(typeof((typeof(var))0) *)&(var)) = (val))
>>> +# endif
>>> +# define rare_write_begin()    __arch_rare_write_begin()
>>> +# define rare_write_end()      __arch_rare_write_end()
>>> +#endif
>>> +#define rare_write(__var, __val) ({                    \
>>> +       rare_write_begin();                             \
>>> +       __rare_write(__var, __val);                     \
>>> +       rare_write_end();                               \
>>> +       __var;                                          \
>>> +})
>>> +
>>
>> Of course, only after sending this do I realize that the MEMCPY case
>> will need to be further adjusted, since it currently can't take
>> literals. I guess something like this needs to be done:
>>
>> #define __rare_write(var, val) ({ \
>>    typeof(var) __src = (val);     \
>>    __rare_write_n(&(var), &(__src), sizeof(var)); \
>> })
>>
>
> Right, and it has a problem with BUILD_BUG, which causes compilation error when CONFIG_HABE_ARCH_RARE_WRITE_MEMCPY is true
> BUILD_BUG is defined in <linux/bug.h> but <linux/bug.h> includes <linux/compiler.h>
>
> Please see the following.
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 3334fa9..3fa50e1 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -350,11 +350,11 @@ static __always_inline void __write_once_size(volatile vo\
> id *p, void *res, int s
>  # define __wr_rare                     __ro_after_init
>  # define __wr_rare_type                        const
>  # ifdef CONFIG_HAVE_ARCH_RARE_WRITE_MEMCPY
> -#  define __rare_write_n(dst, src, len)        ({                      \
> -               BUILD_BUG(!builtin_const(len));                         \
> -               __arch_rare_write_memcpy((dst), (src), (len));   \
> +#  define __rare_write_n(var, val, len)        ({                                 \
> +               typeof(val) __val = val;                                                    \
> +               __arch_rare_write_memcpy(&(var), &(__val), (len));      \
>         })
> -#  define __rare_write(var, val)  __rare_write_n(&(var), &(val), sizeof(var))
> +#  define __rare_write(var, val)  __rare_write_n((var), (val), sizeof(var))
>  # else
>  #  define __rare_write(var, val)  ((*(typeof((typeof(var))0) *)&(var)) = (val)\
> )
>  # endif

Thanks for the catch, I'll update this (I'll use compiletime_assert,
which is defined in compiler.h).

-Kees

-- 
Kees Cook
Pixel Security

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


#1618575 — Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure

FromHo-Eun Ryu <hoeun.ryu@gmail.com>
Date2017-04-07 10:20 +0200
SubjectRe: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure
Message-ID<ttxhF-4AV-39@gated-at.bofh.it>
In reply to#1612229
> On 30 Mar 2017, at 3:15 AM, Kees Cook <keescook@chromium.org> wrote:
> 
> Several types of data storage exist in the kernel: read-write data (.data,
> .bss), read-only data (.rodata), and RO-after-init. This introduces the
> infrastructure for another type: write-rarely, which is intended for data
> that is either only rarely modified or especially security-sensitive. The
> goal is to further reduce the internal attack surface of the kernel by
> making this storage read-only when "at rest". This makes it much harder
> to be subverted by attackers who have a kernel-write flaw, since they
> cannot directly change these memory contents.
> 
> This work is heavily based on PaX and grsecurity's pax_{open,close}_kernel
> API, its __read_only annotations, its constify plugin, and the work done
> to identify sensitive structures that should be moved from .data into
> .rodata. This builds the initial infrastructure to support these kinds
> of changes, though the API and naming has been adjusted in places for
> clarity and maintainability.
> 
> Variables declared with the __wr_rare annotation will be moved to the
> .rodata section if an architecture supports CONFIG_HAVE_ARCH_WRITE_RARE.
> To change these variables, either a single rare_write() macro can be used,
> or multiple uses of __rare_write(), wrapped in a matching pair of
> rare_write_begin() and rare_write_end() macros can be used. These macros
> are expanded into the arch-specific functions that perform the actions
> needed to write to otherwise read-only memory.
> 
> As detailed in the Kconfig help, the arch-specific helpers have several
> requirements to make them sensible/safe for use by the kernel: they must
> not allow non-current CPUs to write the memory area, they must run
> non-preemptible to avoid accidentally leaving memory writable, and must
> be inline to avoid making them desirable ROP targets for attackers.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> arch/Kconfig             | 25 +++++++++++++++++++++++++
> include/linux/compiler.h | 32 ++++++++++++++++++++++++++++++++
> include/linux/preempt.h  |  6 ++++--
> 3 files changed, 61 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index cd211a14a88f..5ebf62500b99 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -847,4 +847,29 @@ config STRICT_MODULE_RWX
> config ARCH_WANT_RELAX_ORDER
> 	bool
> 
> +config HAVE_ARCH_RARE_WRITE
> +	def_bool n
> +	help
> +	  An arch should select this option if it has defined the functions
> +	  __arch_rare_write_begin() and __arch_rare_write_end() to
> +	  respectively enable and disable writing to read-only memory. The
> +	  routines must meet the following requirements:
> +	  - read-only memory writing must only be available on the current
> +	    CPU (to make sure other CPUs can't race to make changes too).
> +	  - the routines must be declared inline (to discourage ROP use).
> +	  - the routines must not be preemptible (likely they will call
> +	    preempt_disable() and preempt_enable_no_resched() respectively).
> +	  - the routines must validate expected state (e.g. when enabling
> +	    writes, BUG() if writes are already be enabled).
> +
> +config HAVE_ARCH_RARE_WRITE_MEMCPY
> +	def_bool n
> +	depends on HAVE_ARCH_RARE_WRITE
> +	help
> +	  An arch should select this option if a special accessor is needed
> +	  to write to otherwise read-only memory, defined by the function
> +	  __arch_rare_write_memcpy(). Without this, the write-rarely
> +	  infrastructure will just attempt to write directly to the memory
> +	  using a const-ignoring assignment.
> +
> source "kernel/gcov/Kconfig"
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index f8110051188f..274bd03cfe9e 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -336,6 +336,38 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
> 	__u.__val;					\
> })
> 
> +/*
> + * Build "write rarely" infrastructure for flipping memory r/w
> + * on a per-CPU basis.
> + */
> +#ifndef CONFIG_HAVE_ARCH_RARE_WRITE
> +# define __wr_rare
> +# define __wr_rare_type
> +# define __rare_write(__var, __val)	(__var = (__val))
> +# define rare_write_begin()		do { } while (0)
> +# define rare_write_end()		do { } while (0)
> +#else
> +# define __wr_rare			__ro_after_init
> +# define __wr_rare_type			const
> +# ifdef CONFIG_HAVE_ARCH_RARE_WRITE_MEMCPY
> +#  define __rare_write_n(dst, src, len)	({			\
> +		BUILD_BUG(!builtin_const(len));			\
> +		__arch_rare_write_memcpy((dst), (src), (len));	\
> +	})
> +#  define __rare_write(var, val)  __rare_write_n(&(var), &(val), sizeof(var))
> +# else
> +#  define __rare_write(var, val)  ((*(typeof((typeof(var))0) *)&(var)) = (val))
> +# endif
> +# define rare_write_begin()	__arch_rare_write_begin()
> +# define rare_write_end()	__arch_rare_write_end()
> +#endif
> +#define rare_write(__var, __val) ({			\
> +	rare_write_begin();				\
> +	__rare_write(__var, __val);			\
> +	rare_write_end();				\
> +	__var;						\
> +})
> +

How about we have a separate header file splitting section annotations and the actual APIs.

include/linux/compiler.h:
    __wr_rare
    __wr_rare_type

include/linux/rare_write.h:
    __rare_write_n()
    __rare_write()
    rare_write_begin()
    rare_write_end()

OR moving all of them to include/linux/rare_write.h.

I’m writing the arm64 port for rare_write feature and I’ve stucked in some header problems for the next version of the patch.
I need some other mmu related APIs (mostly defined in `arch/arm64/include/asm/mmu_context.h`) to implement those helpers.
but I cannot include the header in `include/linux/compiler.h` prior to definition of rare_write macros (huge compilation errors).
You know that `linux/compiler.h` header is mostly base of other headers not a user.

I have to define `__arch_rare_write_[begin/end/memcpy]()` functions as `static inline` in a header file somewhere in `arch/arm64/include/asm/`
to avoid making them ROP targets and the helpers need other mmu related APIs.
And the helpers and the mmu related APIs cannot be defined prior to definition of rare_write macros in `linux/compile.h`.

And I think I'll need `include/linux/module.h` for module related APIs like `is_module_address()` to support module address conversion
in `__arch_rare_write_memcpy()` and it’ll make the situation worse.

* make `include/linux/rare_write.h`
* the header defines rare_write APIs
* the header includes `include/asm/rare_write.h` for arch-specific helpers
* users using rare_write feature should include `linux/rare_write.h`

OR suggest other solutions please.

> #endif /* __KERNEL__ */
> 
> #endif /* __ASSEMBLY__ */
> diff --git a/include/linux/preempt.h b/include/linux/preempt.h
> index cae461224948..4fc97aaa22ea 100644
> --- a/include/linux/preempt.h
> +++ b/include/linux/preempt.h
> @@ -258,10 +258,12 @@ do { \
> /*
>  * Modules have no business playing preemption tricks.
>  */
> -#undef sched_preempt_enable_no_resched
> -#undef preempt_enable_no_resched
> #undef preempt_enable_no_resched_notrace
> #undef preempt_check_resched
> +#ifndef CONFIG_HAVE_ARCH_RARE_WRITE
> +#undef sched_preempt_enable_no_resched
> +#undef preempt_enable_no_resched
> +#endif
> #endif
> 
> #define preempt_set_need_resched() \
> -- 
> 2.7.4
> 

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


#1619085 — Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure

FromKees Cook <keescook@chromium.org>
Date2017-04-07 22:40 +0200
SubjectRe: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure
Message-ID<ttIPL-45V-5@gated-at.bofh.it>
In reply to#1618575
On Fri, Apr 7, 2017 at 1:09 AM, Ho-Eun Ryu <hoeun.ryu@gmail.com> wrote:
>
>> On 30 Mar 2017, at 3:15 AM, Kees Cook <keescook@chromium.org> wrote:
>>
>> Several types of data storage exist in the kernel: read-write data (.data,
>> .bss), read-only data (.rodata), and RO-after-init. This introduces the
>> infrastructure for another type: write-rarely, which is intended for data
>> that is either only rarely modified or especially security-sensitive. The
>> goal is to further reduce the internal attack surface of the kernel by
>> making this storage read-only when "at rest". This makes it much harder
>> to be subverted by attackers who have a kernel-write flaw, since they
>> cannot directly change these memory contents.
>>
>> This work is heavily based on PaX and grsecurity's pax_{open,close}_kernel
>> API, its __read_only annotations, its constify plugin, and the work done
>> to identify sensitive structures that should be moved from .data into
>> .rodata. This builds the initial infrastructure to support these kinds
>> of changes, though the API and naming has been adjusted in places for
>> clarity and maintainability.
>>
>> Variables declared with the __wr_rare annotation will be moved to the
>> .rodata section if an architecture supports CONFIG_HAVE_ARCH_WRITE_RARE.
>> To change these variables, either a single rare_write() macro can be used,
>> or multiple uses of __rare_write(), wrapped in a matching pair of
>> rare_write_begin() and rare_write_end() macros can be used. These macros
>> are expanded into the arch-specific functions that perform the actions
>> needed to write to otherwise read-only memory.
>>
>> As detailed in the Kconfig help, the arch-specific helpers have several
>> requirements to make them sensible/safe for use by the kernel: they must
>> not allow non-current CPUs to write the memory area, they must run
>> non-preemptible to avoid accidentally leaving memory writable, and must
>> be inline to avoid making them desirable ROP targets for attackers.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>> arch/Kconfig             | 25 +++++++++++++++++++++++++
>> include/linux/compiler.h | 32 ++++++++++++++++++++++++++++++++
>> include/linux/preempt.h  |  6 ++++--
>> 3 files changed, 61 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index cd211a14a88f..5ebf62500b99 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -847,4 +847,29 @@ config STRICT_MODULE_RWX
>> config ARCH_WANT_RELAX_ORDER
>>       bool
>>
>> +config HAVE_ARCH_RARE_WRITE
>> +     def_bool n
>> +     help
>> +       An arch should select this option if it has defined the functions
>> +       __arch_rare_write_begin() and __arch_rare_write_end() to
>> +       respectively enable and disable writing to read-only memory. The
>> +       routines must meet the following requirements:
>> +       - read-only memory writing must only be available on the current
>> +         CPU (to make sure other CPUs can't race to make changes too).
>> +       - the routines must be declared inline (to discourage ROP use).
>> +       - the routines must not be preemptible (likely they will call
>> +         preempt_disable() and preempt_enable_no_resched() respectively).
>> +       - the routines must validate expected state (e.g. when enabling
>> +         writes, BUG() if writes are already be enabled).
>> +
>> +config HAVE_ARCH_RARE_WRITE_MEMCPY
>> +     def_bool n
>> +     depends on HAVE_ARCH_RARE_WRITE
>> +     help
>> +       An arch should select this option if a special accessor is needed
>> +       to write to otherwise read-only memory, defined by the function
>> +       __arch_rare_write_memcpy(). Without this, the write-rarely
>> +       infrastructure will just attempt to write directly to the memory
>> +       using a const-ignoring assignment.
>> +
>> source "kernel/gcov/Kconfig"
>> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>> index f8110051188f..274bd03cfe9e 100644
>> --- a/include/linux/compiler.h
>> +++ b/include/linux/compiler.h
>> @@ -336,6 +336,38 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
>>       __u.__val;                                      \
>> })
>>
>> +/*
>> + * Build "write rarely" infrastructure for flipping memory r/w
>> + * on a per-CPU basis.
>> + */
>> +#ifndef CONFIG_HAVE_ARCH_RARE_WRITE
>> +# define __wr_rare
>> +# define __wr_rare_type
>> +# define __rare_write(__var, __val)  (__var = (__val))
>> +# define rare_write_begin()          do { } while (0)
>> +# define rare_write_end()            do { } while (0)
>> +#else
>> +# define __wr_rare                   __ro_after_init
>> +# define __wr_rare_type                      const
>> +# ifdef CONFIG_HAVE_ARCH_RARE_WRITE_MEMCPY
>> +#  define __rare_write_n(dst, src, len)      ({                      \
>> +             BUILD_BUG(!builtin_const(len));                 \
>> +             __arch_rare_write_memcpy((dst), (src), (len));  \
>> +     })
>> +#  define __rare_write(var, val)  __rare_write_n(&(var), &(val), sizeof(var))
>> +# else
>> +#  define __rare_write(var, val)  ((*(typeof((typeof(var))0) *)&(var)) = (val))
>> +# endif
>> +# define rare_write_begin()  __arch_rare_write_begin()
>> +# define rare_write_end()    __arch_rare_write_end()
>> +#endif
>> +#define rare_write(__var, __val) ({                  \
>> +     rare_write_begin();                             \
>> +     __rare_write(__var, __val);                     \
>> +     rare_write_end();                               \
>> +     __var;                                          \
>> +})
>> +
>
> How about we have a separate header file splitting section annotations and the actual APIs.
>
> include/linux/compiler.h:
>     __wr_rare
>     __wr_rare_type
>
> include/linux/rare_write.h:
>     __rare_write_n()
>     __rare_write()
>     rare_write_begin()
>     rare_write_end()

Yeah, that's actually exactly what I did for the current tree:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/write-rarely

-Kees

-- 
Kees Cook
Pixel Security

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


#1612230 — [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

FromKees Cook <keescook@chromium.org>
Date2017-03-29 20:20 +0200
Subject[RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()
Message-ID<tqqmm-2iB-39@gated-at.bofh.it>
In reply to#1612221
Based on PaX's x86 pax_{open,close}_kernel() implementation, this
allows HAVE_ARCH_RARE_WRITE to work on x86.

There is missing work to sort out some header file issues where preempt.h
is missing, though it can't be included in pg_table.h unconditionally...
some other solution will be needed, perhaps an entirely separate header
file for rare_write()-related defines...

This patch is also missing paravirt support.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/Kconfig               |  1 +
 arch/x86/include/asm/pgtable.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc98d5a294ee..2d1d707aa036 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -106,6 +106,7 @@ config X86
 	select HAVE_ARCH_KMEMCHECK
 	select HAVE_ARCH_MMAP_RND_BITS		if MMU
 	select HAVE_ARCH_MMAP_RND_COMPAT_BITS	if MMU && COMPAT
+	select HAVE_ARCH_RARE_WRITE
 	select HAVE_ARCH_SECCOMP_FILTER
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_ARCH_TRANSPARENT_HUGEPAGE
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 1cfb36b8c024..2e6bf661bb84 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -91,6 +91,37 @@ extern struct mm_struct *pgd_page_get_mm(struct page *page);
 
 #endif	/* CONFIG_PARAVIRT */
 
+/* TODO: Bad hack to deal with preempt macros being missing sometimes. */
+#ifndef preempt_disable
+#include <linux/preempt.h>
+#endif
+
+static __always_inline unsigned long __arch_rare_write_begin(void)
+{
+	unsigned long cr0;
+
+	preempt_disable();
+	barrier();
+	cr0 = read_cr0() ^ X86_CR0_WP;
+	BUG_ON(cr0 & X86_CR0_WP);
+	write_cr0(cr0);
+	barrier();
+	return cr0 ^ X86_CR0_WP;
+}
+
+static __always_inline unsigned long __arch_rare_write_end(void)
+{
+	unsigned long cr0;
+
+	barrier();
+	cr0 = read_cr0() ^ X86_CR0_WP;
+	BUG_ON(!(cr0 & X86_CR0_WP));
+	write_cr0(cr0);
+	barrier();
+	preempt_enable_no_resched();
+	return cr0 ^ X86_CR0_WP;
+}
+
 /*
  * The following only work if pte_present() is true.
  * Undefined behaviour if not..
-- 
2.7.4

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


#1612422 — Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

FromAndy Lutomirski <luto@amacapital.net>
Date2017-03-30 00:40 +0200
SubjectRe: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()
Message-ID<tqupX-59N-9@gated-at.bofh.it>
In reply to#1612230
On Wed, Mar 29, 2017 at 11:15 AM, Kees Cook <keescook@chromium.org> wrote:
> Based on PaX's x86 pax_{open,close}_kernel() implementation, this
> allows HAVE_ARCH_RARE_WRITE to work on x86.
>

> +
> +static __always_inline unsigned long __arch_rare_write_begin(void)
> +{
> +       unsigned long cr0;
> +
> +       preempt_disable();

This looks wrong.  DEBUG_LOCKS_WARN_ON(!irqs_disabled()) would work,
as would local_irq_disable().  There's no way that just disabling
preemption is enough.

(Also, how does this interact with perf nmis?)

--Andy

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


#1612490 — Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

FromKees Cook <keescook@chromium.org>
Date2017-03-30 03:50 +0200
SubjectRe: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()
Message-ID<tqxnP-7bX-5@gated-at.bofh.it>
In reply to#1612422
On Wed, Mar 29, 2017 at 3:38 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Wed, Mar 29, 2017 at 11:15 AM, Kees Cook <keescook@chromium.org> wrote:
>> Based on PaX's x86 pax_{open,close}_kernel() implementation, this
>> allows HAVE_ARCH_RARE_WRITE to work on x86.
>>
>
>> +
>> +static __always_inline unsigned long __arch_rare_write_begin(void)
>> +{
>> +       unsigned long cr0;
>> +
>> +       preempt_disable();
>
> This looks wrong.  DEBUG_LOCKS_WARN_ON(!irqs_disabled()) would work,
> as would local_irq_disable().  There's no way that just disabling
> preemption is enough.
>
> (Also, how does this interact with perf nmis?)

Do you mean preempt_disable() isn't strong enough here? I'm open to
suggestions. The goal would be to make sure nothing between _begin and
_end would get executed without interruption...

-Kees

-- 
Kees Cook
Pixel Security

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


#1617430 — Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

FromAndy Lutomirski <luto@kernel.org>
Date2017-04-06 02:00 +0200
SubjectRe: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()
Message-ID<tt30d-NG-7@gated-at.bofh.it>
In reply to#1612490
On Wed, Mar 29, 2017 at 6:41 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Mar 29, 2017 at 3:38 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Wed, Mar 29, 2017 at 11:15 AM, Kees Cook <keescook@chromium.org> wrote:
>>> Based on PaX's x86 pax_{open,close}_kernel() implementation, this
>>> allows HAVE_ARCH_RARE_WRITE to work on x86.
>>>
>>
>>> +
>>> +static __always_inline unsigned long __arch_rare_write_begin(void)
>>> +{
>>> +       unsigned long cr0;
>>> +
>>> +       preempt_disable();
>>
>> This looks wrong.  DEBUG_LOCKS_WARN_ON(!irqs_disabled()) would work,
>> as would local_irq_disable().  There's no way that just disabling
>> preemption is enough.
>>
>> (Also, how does this interact with perf nmis?)
>
> Do you mean preempt_disable() isn't strong enough here? I'm open to
> suggestions. The goal would be to make sure nothing between _begin and
> _end would get executed without interruption...
>

Sorry for the very slow response.

preempt_disable() isn't strong enough to prevent interrupts, and an
interrupt here would run with WP off, causing unknown havoc.  I tend
to think that the caller should be responsible for turning off
interrupts.

--Andy

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


#1617439 — Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

FromKees Cook <keescook@chromium.org>
Date2017-04-06 02:20 +0200
SubjectRe: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()
Message-ID<tt3jz-18Z-7@gated-at.bofh.it>
In reply to#1617430
On Wed, Apr 5, 2017 at 4:57 PM, Andy Lutomirski <luto@kernel.org> wrote:
> On Wed, Mar 29, 2017 at 6:41 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Wed, Mar 29, 2017 at 3:38 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Wed, Mar 29, 2017 at 11:15 AM, Kees Cook <keescook@chromium.org> wrote:
>>>> Based on PaX's x86 pax_{open,close}_kernel() implementation, this
>>>> allows HAVE_ARCH_RARE_WRITE to work on x86.
>>>>
>>>
>>>> +
>>>> +static __always_inline unsigned long __arch_rare_write_begin(void)
>>>> +{
>>>> +       unsigned long cr0;
>>>> +
>>>> +       preempt_disable();
>>>
>>> This looks wrong.  DEBUG_LOCKS_WARN_ON(!irqs_disabled()) would work,
>>> as would local_irq_disable().  There's no way that just disabling
>>> preemption is enough.
>>>
>>> (Also, how does this interact with perf nmis?)
>>
>> Do you mean preempt_disable() isn't strong enough here? I'm open to
>> suggestions. The goal would be to make sure nothing between _begin and
>> _end would get executed without interruption...
>>
>
> Sorry for the very slow response.
>
> preempt_disable() isn't strong enough to prevent interrupts, and an
> interrupt here would run with WP off, causing unknown havoc.  I tend
> to think that the caller should be responsible for turning off
> interrupts.

So, something like:

Top-level functions:

static __always_inline rare_write_begin(void)
{
    preempt_disable();
    local_irq_disable();
    barrier();
    __arch_rare_write_begin();
    barrier();
}

static __always_inline rare_write_end(void)
{
    barrier();
    __arch_rare_write_end();
    barrier();
    local_irq_enable();
    preempt_enable_no_resched();
}

x86-specific helpers:

static __always_inline unsigned long __arch_rare_write_begin(void)
{
       unsigned long cr0;

       cr0 = read_cr0() ^ X86_CR0_WP;
       BUG_ON(cr0 & X86_CR0_WP);
       write_cr0(cr0);
       return cr0 ^ X86_CR0_WP;
}

static __always_inline unsigned long __arch_rare_write_end(void)
{
       unsigned long cr0;

       cr0 = read_cr0() ^ X86_CR0_WP;
       BUG_ON(!(cr0 & X86_CR0_WP));
       write_cr0(cr0);
       return cr0 ^ X86_CR0_WP;
}

I can give it a spin...

-Kees

-- 
Kees Cook
Pixel Security

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


#1618126 — Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

FromAndy Lutomirski <luto@amacapital.net>
Date2017-04-06 18:00 +0200
SubjectRe: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()
Message-ID<tthZg-2Gn-7@gated-at.bofh.it>
In reply to#1617439
On Wed, Apr 5, 2017 at 5:14 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Apr 5, 2017 at 4:57 PM, Andy Lutomirski <luto@kernel.org> wrote:
>> On Wed, Mar 29, 2017 at 6:41 PM, Kees Cook <keescook@chromium.org> wrote:
>>> On Wed, Mar 29, 2017 at 3:38 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> On Wed, Mar 29, 2017 at 11:15 AM, Kees Cook <keescook@chromium.org> wrote:
>>>>> Based on PaX's x86 pax_{open,close}_kernel() implementation, this
>>>>> allows HAVE_ARCH_RARE_WRITE to work on x86.
>>>>>
>>>>
>>>>> +
>>>>> +static __always_inline unsigned long __arch_rare_write_begin(void)
>>>>> +{
>>>>> +       unsigned long cr0;
>>>>> +
>>>>> +       preempt_disable();
>>>>
>>>> This looks wrong.  DEBUG_LOCKS_WARN_ON(!irqs_disabled()) would work,
>>>> as would local_irq_disable().  There's no way that just disabling
>>>> preemption is enough.
>>>>
>>>> (Also, how does this interact with perf nmis?)
>>>
>>> Do you mean preempt_disable() isn't strong enough here? I'm open to
>>> suggestions. The goal would be to make sure nothing between _begin and
>>> _end would get executed without interruption...
>>>
>>
>> Sorry for the very slow response.
>>
>> preempt_disable() isn't strong enough to prevent interrupts, and an
>> interrupt here would run with WP off, causing unknown havoc.  I tend
>> to think that the caller should be responsible for turning off
>> interrupts.
>
> So, something like:
>
> Top-level functions:
>
> static __always_inline rare_write_begin(void)
> {
>     preempt_disable();
>     local_irq_disable();
>     barrier();
>     __arch_rare_write_begin();
>     barrier();
> }

Looks good, except you don't need preempt_disable().
local_irq_disable() also disables preemption.  You might need to use
local_irq_save(), though, depending on whether any callers already
have IRQs off.

--Andy

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


#1618594 — Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

FromMathias Krause <minipli@googlemail.com>
Date2017-04-07 10:40 +0200
SubjectRe: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()
Message-ID<ttxB0-4Hw-21@gated-at.bofh.it>
In reply to#1618126
On 6 April 2017 at 17:59, Andy Lutomirski <luto@amacapital.net> wrote:
> On Wed, Apr 5, 2017 at 5:14 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Wed, Apr 5, 2017 at 4:57 PM, Andy Lutomirski <luto@kernel.org> wrote:
>>> On Wed, Mar 29, 2017 at 6:41 PM, Kees Cook <keescook@chromium.org> wrote:
>>>> On Wed, Mar 29, 2017 at 3:38 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>> On Wed, Mar 29, 2017 at 11:15 AM, Kees Cook <keescook@chromium.org> wrote:
>>>>>> Based on PaX's x86 pax_{open,close}_kernel() implementation, this
>>>>>> allows HAVE_ARCH_RARE_WRITE to work on x86.
>>>>>>
>>>>>> +
>>>>>> +static __always_inline unsigned long __arch_rare_write_begin(void)
>>>>>> +{
>>>>>> +       unsigned long cr0;
>>>>>> +
>>>>>> +       preempt_disable();
>>>>>
>>>>> This looks wrong.  DEBUG_LOCKS_WARN_ON(!irqs_disabled()) would work,
>>>>> as would local_irq_disable().  There's no way that just disabling
>>>>> preemption is enough.
>>>>>
>>>>> (Also, how does this interact with perf nmis?)
>>>>
>>>> Do you mean preempt_disable() isn't strong enough here? I'm open to
>>>> suggestions. The goal would be to make sure nothing between _begin and
>>>> _end would get executed without interruption...
>>>>
>>>
>>> Sorry for the very slow response.
>>>
>>> preempt_disable() isn't strong enough to prevent interrupts, and an
>>> interrupt here would run with WP off, causing unknown havoc.  I tend
>>> to think that the caller should be responsible for turning off
>>> interrupts.
>>
>> So, something like:
>>
>> Top-level functions:
>>
>> static __always_inline rare_write_begin(void)
>> {
>>     preempt_disable();
>>     local_irq_disable();
>>     barrier();
>>     __arch_rare_write_begin();
>>     barrier();
>> }
>
> Looks good, except you don't need preempt_disable().
> local_irq_disable() also disables preemption.  You might need to use
> local_irq_save(), though, depending on whether any callers already
> have IRQs off.

Well, doesn't look good to me. NMIs will still be able to interrupt
this code and will run with CR0.WP = 0.

Shouldn't you instead question yourself why PaX can do it "just" with
preempt_disable() instead?!

Cheers,
Mathias

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


#1618637 — Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

FromThomas Gleixner <tglx@linutronix.de>
Date2017-04-07 11:50 +0200
SubjectRe: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()
Message-ID<ttyGK-5mq-23@gated-at.bofh.it>
In reply to#1618594
On Fri, 7 Apr 2017, Mathias Krause wrote:
> On 6 April 2017 at 17:59, Andy Lutomirski <luto@amacapital.net> wrote:
> > On Wed, Apr 5, 2017 at 5:14 PM, Kees Cook <keescook@chromium.org> wrote:
> >> static __always_inline rare_write_begin(void)
> >> {
> >>     preempt_disable();
> >>     local_irq_disable();
> >>     barrier();
> >>     __arch_rare_write_begin();
> >>     barrier();
> >> }
> >
> > Looks good, except you don't need preempt_disable().
> > local_irq_disable() also disables preemption.  You might need to use
> > local_irq_save(), though, depending on whether any callers already
> > have IRQs off.
> 
> Well, doesn't look good to me. NMIs will still be able to interrupt
> this code and will run with CR0.WP = 0.
> 
> Shouldn't you instead question yourself why PaX can do it "just" with
> preempt_disable() instead?!

That's silly. Just because PaX does it, doesn't mean it's correct. To be
honest, playing games with the CR0.WP bit is outright stupid to begin with.

Whether protected by preempt_disable or local_irq_disable, to make that
work it needs CR0 handling in the exception entry/exit at the lowest
level. And that's just a nightmare maintainence wise as it's prone to be
broken over time. Aside of that it's pointless overhead for the normal case.

The proper solution is:

write_rare(ptr, val)
{
	mp = map_shadow_rw(ptr);
	*mp = val;
	unmap_shadow_rw(mp);
}

map_shadow_rw() is essentially the same thing as we do in the highmem case
where the kernel creates a shadow mapping of the user space pages via
kmap_atomic().

It's valid (at least on x86) to have a shadow map with the same page
attributes but write enabled. That does not require any fixups of CR0 and
just works.

Thanks,

	tglx

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


#1618678 — Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

FromMathias Krause <minipli@googlemail.com>
Date2017-04-07 13:00 +0200
SubjectRe: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()
Message-ID<ttzMu-5ZH-19@gated-at.bofh.it>
In reply to#1618637
On 7 April 2017 at 11:46, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 7 Apr 2017, Mathias Krause wrote:
>> On 6 April 2017 at 17:59, Andy Lutomirski <luto@amacapital.net> wrote:
>> > On Wed, Apr 5, 2017 at 5:14 PM, Kees Cook <keescook@chromium.org> wrote:
>> >> static __always_inline rare_write_begin(void)
>> >> {
>> >>     preempt_disable();
>> >>     local_irq_disable();
>> >>     barrier();
>> >>     __arch_rare_write_begin();
>> >>     barrier();
>> >> }
>> >
>> > Looks good, except you don't need preempt_disable().
>> > local_irq_disable() also disables preemption.  You might need to use
>> > local_irq_save(), though, depending on whether any callers already
>> > have IRQs off.
>>
>> Well, doesn't look good to me. NMIs will still be able to interrupt
>> this code and will run with CR0.WP = 0.
>>
>> Shouldn't you instead question yourself why PaX can do it "just" with
>> preempt_disable() instead?!
>
> That's silly. Just because PaX does it, doesn't mean it's correct. To be
> honest, playing games with the CR0.WP bit is outright stupid to begin with.

Why that? It allows fast and CPU local modifications of r/o memory.
OTOH, an approach that needs to fiddle with page table entries
requires global synchronization to keep the individual TLB states in
sync. Hmm.. Not that fast, I'd say.

> Whether protected by preempt_disable or local_irq_disable, to make that
> work it needs CR0 handling in the exception entry/exit at the lowest
> level. And that's just a nightmare maintainence wise as it's prone to be
> broken over time.

It seems to be working fine for more than a decade now in PaX. So it
can't be such a big maintenance nightmare ;)

>                    Aside of that it's pointless overhead for the normal case.

>
> The proper solution is:
>
> write_rare(ptr, val)
> {
>         mp = map_shadow_rw(ptr);
>         *mp = val;
>         unmap_shadow_rw(mp);
> }
>
> map_shadow_rw() is essentially the same thing as we do in the highmem case
> where the kernel creates a shadow mapping of the user space pages via
> kmap_atomic().

The "proper solution" seems to be much slower compared to just
toggling CR0.WP (which is costly in itself, already) because of the
TLB invalidation / synchronisation involved.

> It's valid (at least on x86) to have a shadow map with the same page
> attributes but write enabled. That does not require any fixups of CR0 and
> just works.

"Just works", sure -- but it's not as tightly focused as the PaX
solution which is CPU local, while your proposed solution is globally
visible.


Cheers,
Mathias

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


#1618786 — Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

FromThomas Gleixner <tglx@linutronix.de>
Date2017-04-07 15:20 +0200
SubjectRe: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()
Message-ID<ttBXX-7zm-5@gated-at.bofh.it>
In reply to#1618678
On Fri, 7 Apr 2017, Mathias Krause wrote:
> On 7 April 2017 at 11:46, Thomas Gleixner <tglx@linutronix.de> wrote:
> > Whether protected by preempt_disable or local_irq_disable, to make that
> > work it needs CR0 handling in the exception entry/exit at the lowest
> > level. And that's just a nightmare maintainence wise as it's prone to be
> > broken over time.
> 
> It seems to be working fine for more than a decade now in PaX. So it
> can't be such a big maintenance nightmare ;)

I really do not care whether PaX wants to chase and verify that over and
over. I certainly don't want to take the chance to leak CR0.WP ever and I
very much care about extra stuff to check in the entry/exit path.

> The "proper solution" seems to be much slower compared to just
> toggling CR0.WP (which is costly in itself, already) because of the
> TLB invalidation / synchronisation involved.

Why the heck should we care about rare writes being performant?

> > It's valid (at least on x86) to have a shadow map with the same page
> > attributes but write enabled. That does not require any fixups of CR0 and
> > just works.
> 
> "Just works", sure -- but it's not as tightly focused as the PaX
> solution which is CPU local, while your proposed solution is globally
> visible.

Making the world and some more writeable hardly qualifies as tightly
focussed. Making the mapping concept CPU local is not rocket science
either. The question is whethers it's worth the trouble.

Thanks,

	tglx

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


Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel


csiph-web