kernel-hardening.lists.openwall.com archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v4 00/12] hardening: statically allocated protected memory
@ 2019-02-11 23:27 Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 01/12] __wr_after_init: Core and default arch Igor Stoppa
                   ` (12 more replies)
  0 siblings, 13 replies; 22+ messages in thread
From: Igor Stoppa @ 2019-02-11 23:27 UTC (permalink / raw)
  Cc: Igor Stoppa, Kees Cook, Ahmed Soliman, linux-integrity,
	Kernel Hardening, Linux-MM, Linux Kernel Mailing List

To: Andy Lutomirski <luto@amacapital.net>,
To: Matthew Wilcox <willy@infradead.org>,
To: Nadav Amit <nadav.amit@gmail.com>
To: Peter Zijlstra <peterz@infradead.org>,
To: Dave Hansen <dave.hansen@linux.intel.com>,
To: Mimi Zohar <zohar@linux.vnet.ibm.com>
To: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Kees Cook <keescook@chromium.org>
CC: Ahmed Soliman <ahmedsoliman@mena.vt.edu>
CC: linux-integrity <linux-integrity@vger.kernel.org>
CC: Kernel Hardening <kernel-hardening@lists.openwall.com>
CC: Linux-MM <linux-mm@kvack.org>
CC: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>

Hello,
at last I'm able to resume work on the memory protection patchset I've
proposed some time ago. This version should address comments received so
far and introduce support for arm64. Details below.

Patch-set implementing write-rare memory protection for statically
allocated data.
Its purpose is to keep write protected the kernel data which is seldom
modified, especially if altering it can be exploited during an attack.

There is no read overhead, however writing requires special operations that
are probably unsuitable for often-changing data.
The use is opt-in, by applying the modifier __wr_after_init to a variable
declaration.

As the name implies, the write protection kicks in only after init() is
completed; before that moment, the data is modifiable in the usual way.

Current Limitations:
* supports only data which is allocated statically, at build time.
* supports only x86_64 and arm64;other architectures need to provide own
  backend

Some notes:
- in case an architecture doesn't support write rare, the behavior is to
  fallback to regular write operations
- before altering any memory, the destination is sanitized
- write rare data is segregated into own set of pages
- only x86_64 and arm64 supported, atm
- the memset_user() assembly functions seems to work, but I'm not too sure
  they are really ok
- I've added a simple example: the protection of ima_policy_flags
- the last patch is optional, but it seemed worth to do the refactoring
- the x86_64 user space address range is double the size of the kernel
  address space, so it's possible to randomize the beginning of the
  mapping of the kernel address space, but on arm64 they have the same
  size, so it's not possible to do the same
- I'm not sure if it's correct, since it doesn't seem to be that common in
  kernel sources, but instead of using #defines for overriding default
  function calls, I'm using "weak" for the default functions.
- unaddressed: Nadav proposed to do:
	#define __wr          __attribute__((address_space(5)))
  but I don't know exactly where to use it atm

Changelog:

v3->v4
------

* added function for setting memory in user space mapping for arm64
* refactored code, to work with both supported architectures
* reduced dependency on x86_64 specific code, to support by default also
  arm64
* improved memset_user() for x86_64, but I'm not sure if I understood
  correctly what was the best way to enhance it.

v2->v3
------

* both wr_memset and wr_memcpy are implemented as generic functions
  the arch code must provide suitable helpers
* regular initialization for ima_policy_flags: it happens during init
* remove spurious code from the initialization function

v1->v2
------

* introduce cleaner split between generic and arch code
* add x86_64 specific memset_user()
* replace kernel-space memset() memcopy() with userspace counterpart
* randomize the base address for the alternate map across the entire
  available address range from user space (128TB - 64TB)
* convert BUG() to WARN()
* turn verification of written data into debugging option
* wr_rcu_assign_pointer() as special case of wr_assign()
* example with protection of ima_policy_flags
* documentation

Igor Stoppa (12):
  __wr_after_init: Core and default arch
  __wr_after_init: x86_64: memset_user()
  __wr_after_init: x86_64: randomize mapping offset
  __wr_after_init: x86_64: enable
  __wr_after_init: arm64: memset_user()
  __wr_after_init: arm64: enable
  __wr_after_init: Documentation: self-protection
  __wr_after_init: lkdtm test
  __wr_after_init: rodata_test: refactor tests
  __wr_after_init: rodata_test: test __wr_after_init
  __wr_after_init: test write rare functionality
  IMA: turn ima_policy_flags into __wr_after_init

 Documentation/security/self-protection.rst |  14 +-
 arch/Kconfig                               |   7 +
 arch/arm64/Kconfig                         |   1 +
 arch/arm64/include/asm/uaccess.h           |   9 ++
 arch/arm64/lib/Makefile                    |   2 +-
 arch/arm64/lib/memset_user.S (new)         |  63 ++++++++
 arch/x86/Kconfig                           |   1 +
 arch/x86/include/asm/uaccess_64.h          |   6 +
 arch/x86/lib/usercopy_64.c                 |  51 ++++++
 arch/x86/mm/Makefile                       |   2 +
 arch/x86/mm/prmem.c (new)                  |  20 +++
 drivers/misc/lkdtm/core.c                  |   3 +
 drivers/misc/lkdtm/lkdtm.h                 |   3 +
 drivers/misc/lkdtm/perms.c                 |  29 ++++
 include/linux/prmem.h (new)                |  71 ++++++++
 mm/Kconfig.debug                           |   8 +
 mm/Makefile                                |   2 +
 mm/prmem.c (new)                           | 179 +++++++++++++++++++++
 mm/rodata_test.c                           |  69 +++++---
 mm/test_write_rare.c (new)                 | 136 ++++++++++++++++
 security/integrity/ima/ima.h               |   3 +-
 security/integrity/ima/ima_policy.c        |   9 +-
 22 files changed, 656 insertions(+), 32 deletions(-)
 create mode 100644 arch/arm64/lib/memset_user.S
 create mode 100644 arch/x86/mm/prmem.c
 create mode 100644 include/linux/prmem.h
 create mode 100644 mm/prmem.c
 create mode 100644 mm/test_write_rare.c

-- 
2.19.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFC PATCH v4 01/12] __wr_after_init: Core and default arch
  2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
@ 2019-02-11 23:27 ` Igor Stoppa
  2019-02-12  2:39   ` Matthew Wilcox
  2019-02-11 23:27 ` [RFC PATCH v4 02/12] __wr_after_init: x86_64: memset_user() Igor Stoppa
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 22+ messages in thread
From: Igor Stoppa @ 2019-02-11 23:27 UTC (permalink / raw)
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Matthew Wilcox,
	Peter Zijlstra, Kees Cook, Dave Hansen, Mimi Zohar,
	Thiago Jung Bauermann, Ahmed Soliman, linux-integrity,
	kernel-hardening, linux-mm, linux-kernel

The patch provides:
- the core functionality for write-rare after init for statically
  allocated data, based on code from Matthew Wilcox
- the default implementation for generic architecture
  A specific architecture can override one or more of the default
  functions.

The core (API) functions are:
- wr_memset(): write rare counterpart of memset()
- wr_memcpy(): write rare counterpart of memcpy()
- wr_assign(): write rare counterpart of the assignment ('=') operator
- wr_rcu_assign_pointer(): write rare counterpart of rcu_assign_pointer()

In case either the selected architecture doesn't support write rare
after init, or the functionality is disabled, the write rare functions
will resolve into their non-write rare counterpart:
- memset()
- memcpy()
- assignment operator
- rcu_assign_pointer()

For code that can be either link as module or as built-in (ex: device
driver init function), it is not possible to tell upfront what will be the
case. For this scenario if the functions are called during system init,
they will automatically choose, at runtime, to go through the fast path of
non-write rare. Should they be invoked later, during module init, they
will use the write-rare path.

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>

CC: Andy Lutomirski <luto@amacapital.net>
CC: Nadav Amit <nadav.amit@gmail.com>
CC: Matthew Wilcox <willy@infradead.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Kees Cook <keescook@chromium.org>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Mimi Zohar <zohar@linux.vnet.ibm.com>
CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Ahmed Soliman <ahmedsoliman@mena.vt.edu>
CC: linux-integrity@vger.kernel.org
CC: kernel-hardening@lists.openwall.com
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
---
 arch/Kconfig                |   7 ++
 include/linux/prmem.h (new) |  71 +++++++++++++++
 mm/Makefile                 |   1 +
 mm/prmem.c (new)            | 179 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 258 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index b0b6d176f1c1..0380d4a64681 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -814,6 +814,13 @@ config ARCH_HAS_PRMEM
 	  architecture specific symbol stating that the architecture provides
 	  a back-end function for the write rare operation.
 
+config ARCH_HAS_PRMEM_HEADER
+	def_bool n
+	depends on ARCH_HAS_PRMEM
+	help
+	  architecture specific symbol stating that the architecture provides
+	  own specific header back-end for the write rare operation.
+
 config PRMEM
 	bool "Write protect critical data that doesn't need high write speed."
 	depends on ARCH_HAS_PRMEM
diff --git a/include/linux/prmem.h b/include/linux/prmem.h
new file mode 100644
index 000000000000..0e4683c503b9
--- /dev/null
+++ b/include/linux/prmem.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * prmem.h: Header for memory protection library - generic part
+ *
+ * (C) Copyright 2018-2019 Huawei Technologies Co. Ltd.
+ * Author: Igor Stoppa <igor.stoppa@huawei.com>
+ */
+
+#ifndef _LINUX_PRMEM_H
+#define _LINUX_PRMEM_H
+
+#include <linux/set_memory.h>
+#include <linux/mutex.h>
+
+#ifndef CONFIG_PRMEM
+
+static inline void *wr_memset(void *p, int c, __kernel_size_t n)
+{
+	return memset(p, c, n);
+}
+
+static inline void *wr_memcpy(void *p, const void *q, __kernel_size_t n)
+{
+	return memcpy(p, q, n);
+}
+
+#define wr_assign(var, val)	((var) = (val))
+#define wr_rcu_assign_pointer(p, v)	rcu_assign_pointer(p, v)
+
+#else
+
+#include <linux/mm.h>
+
+void *wr_memset(void *p, int c, __kernel_size_t n);
+void *wr_memcpy(void *p, const void *q, __kernel_size_t n);
+
+/**
+ * wr_assign() - sets a write-rare variable to a specified value
+ * @var: the variable to set
+ * @val: the new value
+ *
+ * Returns: the variable
+ */
+
+#define wr_assign(dst, val) ({			\
+	typeof(dst) tmp = (typeof(dst))val;	\
+						\
+	wr_memcpy(&dst, &tmp, sizeof(dst));	\
+	dst;					\
+})
+
+/**
+ * wr_rcu_assign_pointer() - initialize a pointer in rcu mode
+ * @p: the rcu pointer - it MUST be aligned to a machine word
+ * @v: the new value
+ *
+ * Returns the value assigned to the rcu pointer.
+ *
+ * It is provided as macro, to match rcu_assign_pointer()
+ * The rcu_assign_pointer() is implemented as equivalent of:
+ *
+ * smp_mb();
+ * WRITE_ONCE();
+ */
+#define wr_rcu_assign_pointer(p, v) ({	\
+	smp_mb();			\
+	wr_assign(p, v);		\
+	p;				\
+})
+#endif
+#endif
diff --git a/mm/Makefile b/mm/Makefile
index d210cc9d6f80..ef3867c16ce0 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_SPARSEMEM)	+= sparse.o
 obj-$(CONFIG_SPARSEMEM_VMEMMAP) += sparse-vmemmap.o
 obj-$(CONFIG_SLOB) += slob.o
 obj-$(CONFIG_MMU_NOTIFIER) += mmu_notifier.o
+obj-$(CONFIG_PRMEM) += prmem.o
 obj-$(CONFIG_KSM) += ksm.o
 obj-$(CONFIG_PAGE_POISONING) += page_poison.o
 obj-$(CONFIG_SLAB) += slab.o
diff --git a/mm/prmem.c b/mm/prmem.c
new file mode 100644
index 000000000000..9383b7d6951e
--- /dev/null
+++ b/mm/prmem.c
@@ -0,0 +1,179 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * prmem.c: Memory Protection Library
+ *
+ * (C) Copyright 2018-2019 Huawei Technologies Co. Ltd.
+ * Author: Igor Stoppa <igor.stoppa@huawei.com>
+ */
+
+#include <linux/mmu_context.h>
+#include <linux/uaccess.h>
+
+/*
+ * In case an architecture needs a different declaration of struct
+ * wr_state, it can select ARCH_HAS_PRMEM_HEADER and provide its own
+ * version, accompanied by matching __wr_enable() and __wr_disable()
+ */
+#ifdef CONFIG_ARCH_HAS_PRMEM_HEADER
+#include <asm/prmem.h>
+#else
+
+struct wr_state {
+	struct mm_struct *prev;
+};
+
+#endif
+
+
+__ro_after_init struct mm_struct *wr_mm;
+__ro_after_init unsigned long wr_base;
+
+/*
+ * Default implementation of arch-specific functionality.
+ * Each arch can override the parts that require special handling.
+ */
+unsigned long __init __weak __init_wr_base(void)
+{
+	return 0UL;
+}
+
+void * __weak __wr_addr(void *addr)
+{
+	return (void *)(wr_base + (unsigned long)addr);
+}
+
+void __weak __wr_enable(struct wr_state *state)
+{
+	lockdep_assert_irqs_disabled();
+	state->prev = current->active_mm;
+	switch_mm_irqs_off(NULL, wr_mm, current);
+}
+
+void __weak __wr_disable(struct wr_state *state)
+{
+	lockdep_assert_irqs_disabled();
+	switch_mm_irqs_off(NULL, state->prev, current);
+}
+
+bool __init __weak __wr_map_address(unsigned long addr)
+{
+	spinlock_t *ptl;
+	pte_t pte;
+	pte_t *ptep;
+	unsigned long wr_addr;
+	struct page *page = virt_to_page(addr);
+
+	if (unlikely(!page))
+		return false;
+	wr_addr = (unsigned long)__wr_addr((void *)addr);
+
+	/* The lock is not needed, but avoids open-coding. */
+	ptep = get_locked_pte(wr_mm, wr_addr, &ptl);
+	if (unlikely(!ptep))
+		return false;
+
+	pte = mk_pte(page, PAGE_KERNEL);
+	set_pte_at(wr_mm, wr_addr, ptep, pte);
+	spin_unlock(ptl);
+	return true;
+}
+
+void * __weak __wr_memset(void *p, int c, __kernel_size_t n)
+{
+	return (void *)memset_user((void __user *)p, (u8)c, n);
+}
+
+void * __weak __wr_memcpy(void *p, const void *q, __kernel_size_t n)
+{
+	return (void *)copy_to_user((void __user *)p, q, n);
+}
+
+/*
+ * The following two variables are statically allocated by the linker
+ * script at the boundaries of the memory region (rounded up to
+ * multiples of PAGE_SIZE) reserved for __wr_after_init.
+ */
+extern long __start_wr_after_init;
+extern long __end_wr_after_init;
+static unsigned long start = (unsigned long)&__start_wr_after_init;
+static unsigned long end = (unsigned long)&__end_wr_after_init;
+static inline bool is_wr_after_init(void *p, __kernel_size_t n)
+{
+	unsigned long low = (unsigned long)p;
+	unsigned long high = low + n;
+
+	return likely(start <= low && high <= end);
+}
+
+#define wr_mem_is_writable() (system_state == SYSTEM_BOOTING)
+
+/**
+ * wr_memcpy() - copies n bytes from q to p
+ * @p: beginning of the memory to write to
+ * @q: beginning of the memory to read from
+ * @n: amount of bytes to copy
+ *
+ * Returns pointer to the destination
+ */
+void *wr_memcpy(void *p, const void *q, __kernel_size_t n)
+{
+	struct wr_state state;
+	void *wr_addr;
+
+	if (WARN_ONCE(!is_wr_after_init(p, n), "Invalid WR range."))
+		return p;
+
+	if (unlikely(wr_mem_is_writable()))
+		return memcpy(p, q, n);
+
+	wr_addr = __wr_addr(p);
+	local_irq_disable();
+	__wr_enable(&state);
+	__wr_memcpy(wr_addr, q, n);
+	__wr_disable(&state);
+	local_irq_enable();
+	return p;
+}
+
+/**
+ * wr_memset() - sets n bytes of the destination p to the c value
+ * @p: beginning of the memory to write to
+ * @c: byte to replicate
+ * @n: amount of bytes to copy
+ *
+ * Returns pointer to the destination
+ */
+void *wr_memset(void *p, int c, __kernel_size_t n)
+{
+	struct wr_state state;
+	void *wr_addr;
+
+	if (WARN_ONCE(!is_wr_after_init(p, n), "Invalid WR range."))
+		return p;
+
+	if (unlikely(wr_mem_is_writable()))
+		return memset(p, c, n);
+
+	wr_addr = __wr_addr(p);
+	local_irq_disable();
+	__wr_enable(&state);
+	__wr_memset(wr_addr, c, n);
+	__wr_disable(&state);
+	local_irq_enable();
+	return p;
+}
+
+struct mm_struct *copy_init_mm(void);
+void __init wr_init(void)
+{
+	unsigned long addr;
+
+	wr_mm = copy_init_mm();
+	BUG_ON(!wr_mm);
+
+	wr_base = __init_wr_base();
+
+	/* Create alternate mapping for the entire wr_after_init range. */
+	for (addr = start; addr < end; addr += PAGE_SIZE)
+		BUG_ON(!__wr_map_address(addr));
+}
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v4 02/12] __wr_after_init: x86_64: memset_user()
  2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 01/12] __wr_after_init: Core and default arch Igor Stoppa
@ 2019-02-11 23:27 ` Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 03/12] __wr_after_init: x86_64: randomize mapping offset Igor Stoppa
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Igor Stoppa @ 2019-02-11 23:27 UTC (permalink / raw)
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Matthew Wilcox,
	Peter Zijlstra, Kees Cook, Dave Hansen, Mimi Zohar,
	Thiago Jung Bauermann, Ahmed Soliman, linux-integrity,
	kernel-hardening, linux-mm, linux-kernel

x86_64 specific version of memset() for user space, memset_user()

In the __wr_after_init scenario, write-rare variables have:
- a primary read-only mapping in kernel memory space
- an alternate, writable mapping, implemented as user-space mapping

The write rare implementation expects the arch code to privide a
memset_user() function, which is currently missing.

clear_user() is the base for memset_user()

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>

CC: Andy Lutomirski <luto@amacapital.net>
CC: Nadav Amit <nadav.amit@gmail.com>
CC: Matthew Wilcox <willy@infradead.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Kees Cook <keescook@chromium.org>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Mimi Zohar <zohar@linux.vnet.ibm.com>
CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Ahmed Soliman <ahmedsoliman@mena.vt.edu>
CC: linux-integrity@vger.kernel.org
CC: kernel-hardening@lists.openwall.com
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/include/asm/uaccess_64.h |  6 ++++
 arch/x86/lib/usercopy_64.c        | 51 +++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h
index a9d637bc301d..f194bfce4866 100644
--- a/arch/x86/include/asm/uaccess_64.h
+++ b/arch/x86/include/asm/uaccess_64.h
@@ -213,4 +213,10 @@ copy_user_handle_tail(char *to, char *from, unsigned len);
 unsigned long
 mcsafe_handle_tail(char *to, char *from, unsigned len);
 
+unsigned long __must_check
+memset_user(void __user *mem, int c, unsigned long len);
+
+unsigned long __must_check
+__memset_user(void __user *mem, int c, unsigned long len);
+
 #endif /* _ASM_X86_UACCESS_64_H */
diff --git a/arch/x86/lib/usercopy_64.c b/arch/x86/lib/usercopy_64.c
index ee42bb0cbeb3..e61963585354 100644
--- a/arch/x86/lib/usercopy_64.c
+++ b/arch/x86/lib/usercopy_64.c
@@ -9,6 +9,57 @@
 #include <linux/uaccess.h>
 #include <linux/highmem.h>
 
+/*
+ * Memset Userspace
+ */
+
+unsigned long __memset_user(void __user *addr, int c, unsigned long size)
+{
+	long __d0;
+	unsigned long  pattern = 0x0101010101010101UL * (0xFFUL & c);
+
+	might_fault();
+	/* no memory constraint: gcc doesn't know about this memory */
+	stac();
+	asm volatile(
+		"	movq %[pattern], %%rdx\n"
+		"	testq  %[size8],%[size8]\n"
+		"	jz     4f\n"
+		"0:	mov %%rdx,(%[dst])\n"
+		"	addq   $8,%[dst]\n"
+		"	decl %%ecx ; jnz   0b\n"
+		"4:	movq  %[size1],%%rcx\n"
+		"	testl %%ecx,%%ecx\n"
+		"	jz     2f\n"
+		"1:	movb   %%dl,(%[dst])\n"
+		"	incq   %[dst]\n"
+		"	decl %%ecx ; jnz  1b\n"
+		"2:\n"
+		".section .fixup,\"ax\"\n"
+		"3:	lea 0(%[size1],%[size8],8),%[size8]\n"
+		"	jmp 2b\n"
+		".previous\n"
+		_ASM_EXTABLE_UA(0b, 3b)
+		_ASM_EXTABLE_UA(1b, 2b)
+		: [size8] "=&c"(size), [dst] "=&D" (__d0)
+		: [size1] "r" (size & 7), "[size8]" (size / 8),
+		  "[dst]" (addr), [pattern] "r" (pattern)
+		: "rdx");
+
+	clac();
+	return size;
+}
+EXPORT_SYMBOL(__memset_user);
+
+unsigned long memset_user(void __user *to, int c, unsigned long n)
+{
+	if (access_ok(to, n))
+		return __memset_user(to, c, n);
+	return n;
+}
+EXPORT_SYMBOL(memset_user);
+
+
 /*
  * Zero Userspace
  */
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v4 03/12] __wr_after_init: x86_64: randomize mapping offset
  2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 01/12] __wr_after_init: Core and default arch Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 02/12] __wr_after_init: x86_64: memset_user() Igor Stoppa
@ 2019-02-11 23:27 ` Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 04/12] __wr_after_init: x86_64: enable Igor Stoppa
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Igor Stoppa @ 2019-02-11 23:27 UTC (permalink / raw)
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Matthew Wilcox,
	Peter Zijlstra, Kees Cook, Dave Hansen, Mimi Zohar,
	Thiago Jung Bauermann, Ahmed Soliman, linux-integrity,
	kernel-hardening, linux-mm, linux-kernel

x86_64 specialized way of defining the base address for the alternate
mapping used by write-rare.

Since the kernel address space spans across 64TB and it is mapped into a
used address space of 128TB, the kernel address space can be shifted by a
random offset that is up to 64TB and page aligned.

This is accomplished by providing arch-specific version of the function
__init_wr_base()

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>

CC: Andy Lutomirski <luto@amacapital.net>
CC: Nadav Amit <nadav.amit@gmail.com>
CC: Matthew Wilcox <willy@infradead.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Kees Cook <keescook@chromium.org>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Mimi Zohar <zohar@linux.vnet.ibm.com>
CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Ahmed Soliman <ahmedsoliman@mena.vt.edu>
CC: linux-integrity@vger.kernel.org
CC: kernel-hardening@lists.openwall.com
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/mm/Makefile      |  2 ++
 arch/x86/mm/prmem.c (new) | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile
index 4b101dd6e52f..66652de1e2c7 100644
--- a/arch/x86/mm/Makefile
+++ b/arch/x86/mm/Makefile
@@ -53,3 +53,5 @@ obj-$(CONFIG_PAGE_TABLE_ISOLATION)		+= pti.o
 obj-$(CONFIG_AMD_MEM_ENCRYPT)	+= mem_encrypt.o
 obj-$(CONFIG_AMD_MEM_ENCRYPT)	+= mem_encrypt_identity.o
 obj-$(CONFIG_AMD_MEM_ENCRYPT)	+= mem_encrypt_boot.o
+
+obj-$(CONFIG_PRMEM)		+= prmem.o
diff --git a/arch/x86/mm/prmem.c b/arch/x86/mm/prmem.c
new file mode 100644
index 000000000000..b04fc03f92fb
--- /dev/null
+++ b/arch/x86/mm/prmem.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * prmem.c: Memory Protection Library - x86_64 backend
+ *
+ * (C) Copyright 2018-2019 Huawei Technologies Co. Ltd.
+ * Author: Igor Stoppa <igor.stoppa@huawei.com>
+ */
+
+#include <linux/mm.h>
+#include <linux/mmu_context.h>
+
+unsigned long __init __init_wr_base(void)
+{
+	/*
+	 * Place 64TB of kernel address space within 128TB of user address
+	 * space, at a random page aligned offset.
+	 */
+	return (((unsigned long)kaslr_get_random_long("WR Poke")) &
+		PAGE_MASK) % (64 * _BITUL(40));
+}
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v4 04/12] __wr_after_init: x86_64: enable
  2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
                   ` (2 preceding siblings ...)
  2019-02-11 23:27 ` [RFC PATCH v4 03/12] __wr_after_init: x86_64: randomize mapping offset Igor Stoppa
@ 2019-02-11 23:27 ` Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 05/12] __wr_after_init: arm64: memset_user() Igor Stoppa
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Igor Stoppa @ 2019-02-11 23:27 UTC (permalink / raw)
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Matthew Wilcox,
	Peter Zijlstra, Kees Cook, Dave Hansen, Mimi Zohar,
	Thiago Jung Bauermann, Ahmed Soliman, linux-integrity,
	kernel-hardening, linux-mm, linux-kernel

Set ARCH_HAS_PRMEM to Y for x86_64

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>

CC: Andy Lutomirski <luto@amacapital.net>
CC: Nadav Amit <nadav.amit@gmail.com>
CC: Matthew Wilcox <willy@infradead.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Kees Cook <keescook@chromium.org>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Mimi Zohar <zohar@linux.vnet.ibm.com>
CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Ahmed Soliman <ahmedsoliman@mena.vt.edu>
CC: linux-integrity@vger.kernel.org
CC: kernel-hardening@lists.openwall.com
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 68261430fe6e..7392b53b12c2 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -32,6 +32,7 @@ config X86_64
 	select SWIOTLB
 	select X86_DEV_DMA_OPS
 	select ARCH_HAS_SYSCALL_WRAPPER
+	select ARCH_HAS_PRMEM
 
 #
 # Arch settings
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v4 05/12] __wr_after_init: arm64: memset_user()
  2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
                   ` (3 preceding siblings ...)
  2019-02-11 23:27 ` [RFC PATCH v4 04/12] __wr_after_init: x86_64: enable Igor Stoppa
@ 2019-02-11 23:27 ` Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 06/12] __wr_after_init: arm64: enable Igor Stoppa
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Igor Stoppa @ 2019-02-11 23:27 UTC (permalink / raw)
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Matthew Wilcox,
	Peter Zijlstra, Kees Cook, Dave Hansen, Mimi Zohar,
	Thiago Jung Bauermann, Ahmed Soliman, linux-integrity,
	kernel-hardening, linux-mm, linux-kernel

arm64 specific version of memset() for user space, memset_user()

In the __wr_after_init scenario, write-rare variables have:
- a primary read-only mapping in kernel memory space
- an alternate, writable mapping, implemented as user-space mapping

The write rare implementation expects the arch code to privide a
memset_user() function, which is currently missing.

clear_user() is the base for memset_user()

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>

CC: Andy Lutomirski <luto@amacapital.net>
CC: Nadav Amit <nadav.amit@gmail.com>
CC: Matthew Wilcox <willy@infradead.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Kees Cook <keescook@chromium.org>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Mimi Zohar <zohar@linux.vnet.ibm.com>
CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Ahmed Soliman <ahmedsoliman@mena.vt.edu>
CC: linux-integrity@vger.kernel.org
CC: kernel-hardening@lists.openwall.com
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
---
 arch/arm64/include/asm/uaccess.h   |  9 +++++
 arch/arm64/lib/Makefile            |  2 +-
 arch/arm64/lib/memset_user.S (new) | 63 ++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 547d7a0c9d05..0094f92a8f1b 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -415,6 +415,15 @@ extern unsigned long __must_check __arch_copy_in_user(void __user *to, const voi
 #define INLINE_COPY_TO_USER
 #define INLINE_COPY_FROM_USER
 
+extern unsigned long __must_check __arch_memset_user(void __user *to, int c, unsigned long n);
+static inline unsigned long __must_check __memset_user(void __user *to, int c, unsigned long n)
+{
+	if (access_ok(to, n))
+		n = __arch_memset_user(__uaccess_mask_ptr(to), c, n);
+	return n;
+}
+#define memset_user	__memset_user
+
 extern unsigned long __must_check __arch_clear_user(void __user *to, unsigned long n);
 static inline unsigned long __must_check __clear_user(void __user *to, unsigned long n)
 {
diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
index 5540a1638baf..614b090888de 100644
--- a/arch/arm64/lib/Makefile
+++ b/arch/arm64/lib/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-lib-y		:= clear_user.o delay.o copy_from_user.o		\
+lib-y		:= clear_user.o memset_user.o delay.o copy_from_user.o	\
 		   copy_to_user.o copy_in_user.o copy_page.o		\
 		   clear_page.o memchr.o memcpy.o memmove.o memset.o	\
 		   memcmp.o strcmp.o strncmp.o strlen.o strnlen.o	\
diff --git a/arch/arm64/lib/memset_user.S b/arch/arm64/lib/memset_user.S
new file mode 100644
index 000000000000..1bfbda3d112b
--- /dev/null
+++ b/arch/arm64/lib/memset_user.S
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * memset_user.S - memset for userspace on arm64
+ *
+ * (C) Copyright 2018 Huawey Technologies Co. Ltd.
+ * Author: Igor Stoppa <igor.stoppa@huawei.com>
+ *
+ * Based on arch/arm64/lib/clear_user.S
+ */
+
+#include <linux/linkage.h>
+
+#include <asm/asm-uaccess.h>
+
+	.text
+
+/* Prototype: int __arch_memset_user(void *addr, int c, size_t n)
+ * Purpose  : set n bytes of user memory at "addr" to the value "c"
+ * Params   : x0 - addr, user memory address to set
+ *          : x1 - c, byte value
+ *          : x2 - n, number of bytes to set
+ * Returns  : number of bytes NOT set
+ *
+ * Alignment fixed up by hardware.
+ */
+ENTRY(__arch_memset_user)
+	uaccess_enable_not_uao x3, x4, x5
+	// replicate the byte to the whole register
+	and	x1, x1, 0xff
+	lsl	x3, x1, 8
+	orr	x1, x3, x1
+	lsl	x3, x1, 16
+	orr 	x1, x3, x1
+	lsl	x3, x1, 32
+	orr	x1, x3, x1
+	mov	x3, x2			// save the size for fixup return
+	subs	x2, x2, #8
+	b.mi	2f
+1:
+uao_user_alternative 9f, str, sttr, x1, x0, 8
+	subs	x2, x2, #8
+	b.pl	1b
+2:	adds	x2, x2, #4
+	b.mi	3f
+uao_user_alternative 9f, str, sttr, x1, x0, 4
+	sub	x2, x2, #4
+3:	adds	x2, x2, #2
+	b.mi	4f
+uao_user_alternative 9f, strh, sttrh, w1, x0, 2
+	sub	x2, x2, #2
+4:	adds	x2, x2, #1
+	b.mi	5f
+uao_user_alternative 9f, strb, sttrb, w1, x0, 0
+5:	mov	x0, #0
+	uaccess_disable_not_uao x3, x4
+	ret
+ENDPROC(__arch_memset_user)
+
+	.section .fixup,"ax"
+	.align	2
+9:	mov	x0, x3			// return the original size
+	ret
+	.previous
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v4 06/12] __wr_after_init: arm64: enable
  2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
                   ` (4 preceding siblings ...)
  2019-02-11 23:27 ` [RFC PATCH v4 05/12] __wr_after_init: arm64: memset_user() Igor Stoppa
@ 2019-02-11 23:27 ` Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 07/12] __wr_after_init: Documentation: self-protection Igor Stoppa
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Igor Stoppa @ 2019-02-11 23:27 UTC (permalink / raw)
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Matthew Wilcox,
	Peter Zijlstra, Kees Cook, Dave Hansen, Mimi Zohar,
	Thiago Jung Bauermann, Ahmed Soliman, linux-integrity,
	kernel-hardening, linux-mm, linux-kernel

Set ARCH_HAS_PRMEM to Y for arm64

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>

CC: Andy Lutomirski <luto@amacapital.net>
CC: Nadav Amit <nadav.amit@gmail.com>
CC: Matthew Wilcox <willy@infradead.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Kees Cook <keescook@chromium.org>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Mimi Zohar <zohar@linux.vnet.ibm.com>
CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Ahmed Soliman <ahmedsoliman@mena.vt.edu>
CC: linux-integrity@vger.kernel.org
CC: kernel-hardening@lists.openwall.com
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
---
 arch/arm64/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a4168d366127..7cbb2c133ed7 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -66,6 +66,7 @@ config ARM64
 	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
 	select ARCH_WANT_FRAME_POINTERS
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
+	select ARCH_HAS_PRMEM
 	select ARM_AMBA
 	select ARM_ARCH_TIMER
 	select ARM_GIC
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v4 07/12] __wr_after_init: Documentation: self-protection
  2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
                   ` (5 preceding siblings ...)
  2019-02-11 23:27 ` [RFC PATCH v4 06/12] __wr_after_init: arm64: enable Igor Stoppa
@ 2019-02-11 23:27 ` Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 08/12] __wr_after_init: lkdtm test Igor Stoppa
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Igor Stoppa @ 2019-02-11 23:27 UTC (permalink / raw)
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Matthew Wilcox,
	Peter Zijlstra, Kees Cook, Dave Hansen, Mimi Zohar,
	Thiago Jung Bauermann, Ahmed Soliman, linux-integrity,
	kernel-hardening, linux-mm, linux-kernel

Update the self-protection documentation, to mention also the use of the
__wr_after_init attribute.

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>

CC: Andy Lutomirski <luto@amacapital.net>
CC: Nadav Amit <nadav.amit@gmail.com>
CC: Matthew Wilcox <willy@infradead.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Kees Cook <keescook@chromium.org>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Mimi Zohar <zohar@linux.vnet.ibm.com>
CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Ahmed Soliman <ahmedsoliman@mena.vt.edu>
CC: linux-integrity@vger.kernel.org
CC: kernel-hardening@lists.openwall.com
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
---
 Documentation/security/self-protection.rst | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/security/self-protection.rst b/Documentation/security/self-protection.rst
index f584fb74b4ff..df2614bc25b9 100644
--- a/Documentation/security/self-protection.rst
+++ b/Documentation/security/self-protection.rst
@@ -84,12 +84,14 @@ For variables that are initialized once at ``__init`` time, these can
 be marked with the (new and under development) ``__ro_after_init``
 attribute.
 
-What remains are variables that are updated rarely (e.g. GDT). These
-will need another infrastructure (similar to the temporary exceptions
-made to kernel code mentioned above) that allow them to spend the rest
-of their lifetime read-only. (For example, when being updated, only the
-CPU thread performing the update would be given uninterruptible write
-access to the memory.)
+Others, which are statically allocated, but still need to be updated
+rarely, can be marked with the ``__wr_after_init`` attribute.
+
+The update mechanism must avoid exposing the data to rogue alterations
+during the update. For example, only the CPU thread performing the update
+would be given uninterruptible write access to the memory.
+
+Currently there is no protection available for data allocated dynamically.
 
 Segregation of kernel memory from userspace memory
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v4 08/12] __wr_after_init: lkdtm test
  2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
                   ` (6 preceding siblings ...)
  2019-02-11 23:27 ` [RFC PATCH v4 07/12] __wr_after_init: Documentation: self-protection Igor Stoppa
@ 2019-02-11 23:27 ` Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 09/12] __wr_after_init: rodata_test: refactor tests Igor Stoppa
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Igor Stoppa @ 2019-02-11 23:27 UTC (permalink / raw)
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Matthew Wilcox,
	Peter Zijlstra, Kees Cook, Dave Hansen, Mimi Zohar,
	Thiago Jung Bauermann, Ahmed Soliman, linux-integrity,
	kernel-hardening, linux-mm, linux-kernel

Verify that trying to modify a variable with the __wr_after_init
attribute will cause a crash.

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>

CC: Andy Lutomirski <luto@amacapital.net>
CC: Nadav Amit <nadav.amit@gmail.com>
CC: Matthew Wilcox <willy@infradead.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Kees Cook <keescook@chromium.org>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Mimi Zohar <zohar@linux.vnet.ibm.com>
CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Ahmed Soliman <ahmedsoliman@mena.vt.edu>
CC: linux-integrity@vger.kernel.org
CC: kernel-hardening@lists.openwall.com
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
---
 drivers/misc/lkdtm/core.c  |  3 +++
 drivers/misc/lkdtm/lkdtm.h |  3 +++
 drivers/misc/lkdtm/perms.c | 29 +++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+)

diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index 2837dc77478e..73c34b17c433 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -155,6 +155,9 @@ static const struct crashtype crashtypes[] = {
 	CRASHTYPE(ACCESS_USERSPACE),
 	CRASHTYPE(WRITE_RO),
 	CRASHTYPE(WRITE_RO_AFTER_INIT),
+#ifdef CONFIG_PRMEM
+	CRASHTYPE(WRITE_WR_AFTER_INIT),
+#endif
 	CRASHTYPE(WRITE_KERN),
 	CRASHTYPE(REFCOUNT_INC_OVERFLOW),
 	CRASHTYPE(REFCOUNT_ADD_OVERFLOW),
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 3c6fd327e166..abba2f52ffa6 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -38,6 +38,9 @@ void lkdtm_READ_BUDDY_AFTER_FREE(void);
 void __init lkdtm_perms_init(void);
 void lkdtm_WRITE_RO(void);
 void lkdtm_WRITE_RO_AFTER_INIT(void);
+#ifdef CONFIG_PRMEM
+void lkdtm_WRITE_WR_AFTER_INIT(void);
+#endif
 void lkdtm_WRITE_KERN(void);
 void lkdtm_EXEC_DATA(void);
 void lkdtm_EXEC_STACK(void);
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 53b85c9d16b8..f681730aa652 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -9,6 +9,7 @@
 #include <linux/vmalloc.h>
 #include <linux/mman.h>
 #include <linux/uaccess.h>
+#include <linux/prmem.h>
 #include <asm/cacheflush.h>
 
 /* Whether or not to fill the target memory area with do_nothing(). */
@@ -27,6 +28,10 @@ static const unsigned long rodata = 0xAA55AA55;
 /* This is marked __ro_after_init, so it should ultimately be .rodata. */
 static unsigned long ro_after_init __ro_after_init = 0x55AA5500;
 
+/* This is marked __wr_after_init, so it should be in .rodata. */
+static
+unsigned long wr_after_init __wr_after_init = 0x55AA5500;
+
 /*
  * This just returns to the caller. It is designed to be copied into
  * non-executable memory regions.
@@ -104,6 +109,28 @@ void lkdtm_WRITE_RO_AFTER_INIT(void)
 	*ptr ^= 0xabcd1234;
 }
 
+#ifdef CONFIG_PRMEM
+
+void lkdtm_WRITE_WR_AFTER_INIT(void)
+{
+	unsigned long *ptr = &wr_after_init;
+
+	/*
+	 * Verify we were written to during init. Since an Oops
+	 * is considered a "success", a failure is to just skip the
+	 * real test.
+	 */
+	if ((*ptr & 0xAA) != 0xAA) {
+		pr_info("%p was NOT written during init!?\n", ptr);
+		return;
+	}
+
+	pr_info("attempting bad wr_after_init write at %p\n", ptr);
+	*ptr ^= 0xabcd1234;
+}
+
+#endif
+
 void lkdtm_WRITE_KERN(void)
 {
 	size_t size;
@@ -200,4 +227,6 @@ void __init lkdtm_perms_init(void)
 	/* Make sure we can write to __ro_after_init values during __init */
 	ro_after_init |= 0xAA;
 
+	/* Make sure we can write to __wr_after_init during __init */
+	wr_after_init |= 0xAA;
 }
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v4 09/12] __wr_after_init: rodata_test: refactor tests
  2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
                   ` (7 preceding siblings ...)
  2019-02-11 23:27 ` [RFC PATCH v4 08/12] __wr_after_init: lkdtm test Igor Stoppa
@ 2019-02-11 23:27 ` Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 10/12] __wr_after_init: rodata_test: test __wr_after_init Igor Stoppa
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Igor Stoppa @ 2019-02-11 23:27 UTC (permalink / raw)
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Matthew Wilcox,
	Peter Zijlstra, Kees Cook, Dave Hansen, Mimi Zohar,
	Thiago Jung Bauermann, Ahmed Soliman, linux-integrity,
	kernel-hardening, linux-mm, linux-kernel

Refactor the test cases, in preparation for using them also for testing
__wr_after_init memory, when available.

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>

CC: Andy Lutomirski <luto@amacapital.net>
CC: Nadav Amit <nadav.amit@gmail.com>
CC: Matthew Wilcox <willy@infradead.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Kees Cook <keescook@chromium.org>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Mimi Zohar <zohar@linux.vnet.ibm.com>
CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Ahmed Soliman <ahmedsoliman@mena.vt.edu>
CC: linux-integrity@vger.kernel.org
CC: kernel-hardening@lists.openwall.com
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
---
 mm/rodata_test.c | 48 ++++++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/mm/rodata_test.c b/mm/rodata_test.c
index d908c8769b48..e1349520b436 100644
--- a/mm/rodata_test.c
+++ b/mm/rodata_test.c
@@ -14,44 +14,52 @@
 #include <linux/uaccess.h>
 #include <asm/sections.h>
 
-static const int rodata_test_data = 0xC3;
+#define INIT_TEST_VAL 0xC3
 
-void rodata_test(void)
+static const int rodata_test_data = INIT_TEST_VAL;
+
+static bool test_data(char *data_type, const int *data,
+		      unsigned long start, unsigned long end)
 {
-	unsigned long start, end;
 	int zero = 0;
 
 	/* test 1: read the value */
 	/* If this test fails, some previous testrun has clobbered the state */
-	if (!rodata_test_data) {
-		pr_err("test 1 fails (start data)\n");
-		return;
+	if (*data != INIT_TEST_VAL) {
+		pr_err("%s: test 1 fails (init data value)\n", data_type);
+		return false;
 	}
 
 	/* test 2: write to the variable; this should fault */
-	if (!probe_kernel_write((void *)&rodata_test_data,
-				(void *)&zero, sizeof(zero))) {
-		pr_err("test data was not read only\n");
-		return;
+	if (!probe_kernel_write((void *)data, (void *)&zero, sizeof(zero))) {
+		pr_err("%s: test data was not read only\n", data_type);
+		return false;
 	}
 
 	/* test 3: check the value hasn't changed */
-	if (rodata_test_data == zero) {
-		pr_err("test data was changed\n");
-		return;
+	if (*data != INIT_TEST_VAL) {
+		pr_err("%s: test data was changed\n", data_type);
+		return false;
 	}
 
 	/* test 4: check if the rodata section is PAGE_SIZE aligned */
-	start = (unsigned long)__start_rodata;
-	end = (unsigned long)__end_rodata;
 	if (start & (PAGE_SIZE - 1)) {
-		pr_err("start of .rodata is not page size aligned\n");
-		return;
+		pr_err("%s: start of data is not page size aligned\n",
+		       data_type);
+		return false;
 	}
 	if (end & (PAGE_SIZE - 1)) {
-		pr_err("end of .rodata is not page size aligned\n");
-		return;
+		pr_err("%s: end of data is not page size aligned\n",
+		       data_type);
+		return false;
 	}
+	pr_info("%s tests were successful", data_type);
+	return true;
+}
 
-	pr_info("all tests were successful\n");
+void rodata_test(void)
+{
+	test_data("rodata", &rodata_test_data,
+		  (unsigned long)&__start_rodata,
+		  (unsigned long)&__end_rodata);
 }
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v4 10/12] __wr_after_init: rodata_test: test __wr_after_init
  2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
                   ` (8 preceding siblings ...)
  2019-02-11 23:27 ` [RFC PATCH v4 09/12] __wr_after_init: rodata_test: refactor tests Igor Stoppa
@ 2019-02-11 23:27 ` Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 11/12] __wr_after_init: test write rare functionality Igor Stoppa
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Igor Stoppa @ 2019-02-11 23:27 UTC (permalink / raw)
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Matthew Wilcox,
	Peter Zijlstra, Kees Cook, Dave Hansen, Mimi Zohar,
	Thiago Jung Bauermann, Ahmed Soliman, linux-integrity,
	kernel-hardening, linux-mm, linux-kernel

The write protection of the __wr_after_init data can be verified with the
same methodology used for const data.

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>

CC: Andy Lutomirski <luto@amacapital.net>
CC: Nadav Amit <nadav.amit@gmail.com>
CC: Matthew Wilcox <willy@infradead.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Kees Cook <keescook@chromium.org>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Mimi Zohar <zohar@linux.vnet.ibm.com>
CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Ahmed Soliman <ahmedsoliman@mena.vt.edu>
CC: linux-integrity@vger.kernel.org
CC: kernel-hardening@lists.openwall.com
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
---
 mm/rodata_test.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/mm/rodata_test.c b/mm/rodata_test.c
index e1349520b436..a669cf9f5a61 100644
--- a/mm/rodata_test.c
+++ b/mm/rodata_test.c
@@ -16,8 +16,23 @@
 
 #define INIT_TEST_VAL 0xC3
 
+/*
+ * Note: __ro_after_init data is, for every practical effect, equivalent to
+ * const data, since they are even write protected at the same time; there
+ * is no need for separate testing.
+ * __wr_after_init data, otoh, is altered also after the write protection
+ * takes place and it cannot be exploitable for altering more permanent
+ * data.
+ */
+
 static const int rodata_test_data = INIT_TEST_VAL;
 
+#ifdef CONFIG_PRMEM
+static int wr_after_init_test_data __wr_after_init = INIT_TEST_VAL;
+extern long __start_wr_after_init;
+extern long __end_wr_after_init;
+#endif
+
 static bool test_data(char *data_type, const int *data,
 		      unsigned long start, unsigned long end)
 {
@@ -59,7 +74,13 @@ static bool test_data(char *data_type, const int *data,
 
 void rodata_test(void)
 {
-	test_data("rodata", &rodata_test_data,
-		  (unsigned long)&__start_rodata,
-		  (unsigned long)&__end_rodata);
+	if (!test_data("rodata", &rodata_test_data,
+		       (unsigned long)&__start_rodata,
+		       (unsigned long)&__end_rodata))
+		return;
+#ifdef CONFIG_PRMEM
+	    test_data("wr after init data", &wr_after_init_test_data,
+		      (unsigned long)&__start_wr_after_init,
+		      (unsigned long)&__end_wr_after_init);
+#endif
 }
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v4 11/12] __wr_after_init: test write rare functionality
  2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
                   ` (9 preceding siblings ...)
  2019-02-11 23:27 ` [RFC PATCH v4 10/12] __wr_after_init: rodata_test: test __wr_after_init Igor Stoppa
@ 2019-02-11 23:27 ` Igor Stoppa
  2019-02-11 23:27 ` [RFC PATCH v4 12/12] IMA: turn ima_policy_flags into __wr_after_init Igor Stoppa
  2019-02-12  0:09 ` [RFC PATCH v4 00/12] hardening: statically allocated protected memory Kees Cook
  12 siblings, 0 replies; 22+ messages in thread
From: Igor Stoppa @ 2019-02-11 23:27 UTC (permalink / raw)
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Matthew Wilcox,
	Peter Zijlstra, Kees Cook, Dave Hansen, Mimi Zohar,
	Thiago Jung Bauermann, Ahmed Soliman, linux-integrity,
	kernel-hardening, linux-mm, linux-kernel

Set of test cases meant to confirm that the write rare functionality
works as expected.
It can be optionally compiled as module.

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>

CC: Andy Lutomirski <luto@amacapital.net>
CC: Nadav Amit <nadav.amit@gmail.com>
CC: Matthew Wilcox <willy@infradead.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Kees Cook <keescook@chromium.org>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Mimi Zohar <zohar@linux.vnet.ibm.com>
CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Ahmed Soliman <ahmedsoliman@mena.vt.edu>
CC: linux-integrity@vger.kernel.org
CC: kernel-hardening@lists.openwall.com
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
---
 mm/Kconfig.debug           |   8 +++
 mm/Makefile                |   1 +
 mm/test_write_rare.c (new) | 136 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+)

diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
index 9a7b8b049d04..a62c31901fea 100644
--- a/mm/Kconfig.debug
+++ b/mm/Kconfig.debug
@@ -94,3 +94,11 @@ config DEBUG_RODATA_TEST
     depends on STRICT_KERNEL_RWX
     ---help---
       This option enables a testcase for the setting rodata read-only.
+
+config DEBUG_PRMEM_TEST
+    tristate "Run self test for statically allocated protected memory"
+    depends on PRMEM
+    default n
+    help
+      Tries to verify that the protection for statically allocated memory
+      works correctly and that the memory is effectively protected.
diff --git a/mm/Makefile b/mm/Makefile
index ef3867c16ce0..8de1d468f4e7 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_SPARSEMEM_VMEMMAP) += sparse-vmemmap.o
 obj-$(CONFIG_SLOB) += slob.o
 obj-$(CONFIG_MMU_NOTIFIER) += mmu_notifier.o
 obj-$(CONFIG_PRMEM) += prmem.o
+obj-$(CONFIG_DEBUG_PRMEM_TEST) += test_write_rare.o
 obj-$(CONFIG_KSM) += ksm.o
 obj-$(CONFIG_PAGE_POISONING) += page_poison.o
 obj-$(CONFIG_SLAB) += slab.o
diff --git a/mm/test_write_rare.c b/mm/test_write_rare.c
new file mode 100644
index 000000000000..dd2a0e2d6024
--- /dev/null
+++ b/mm/test_write_rare.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * test_write_rare.c
+ *
+ * (C) Copyright 2018 Huawei Technologies Co. Ltd.
+ * Author: Igor Stoppa <igor.stoppa@huawei.com>
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/mm.h>
+#include <linux/bug.h>
+#include <linux/string.h>
+#include <linux/prmem.h>
+
+#ifdef pr_fmt
+#undef pr_fmt
+#endif
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+extern long __start_wr_after_init;
+extern long __end_wr_after_init;
+
+static __wr_after_init int scalar = '0';
+static __wr_after_init u8 array[PAGE_SIZE * 3] __aligned(PAGE_SIZE);
+
+/* The section must occupy a non-zero number of whole pages */
+static bool test_alignment(void)
+{
+	unsigned long pstart = (unsigned long)&__start_wr_after_init;
+	unsigned long pend = (unsigned long)&__end_wr_after_init;
+
+	if (WARN((pstart & ~PAGE_MASK) || (pend & ~PAGE_MASK) ||
+		 (pstart >= pend), "Boundaries test failed."))
+		return false;
+	pr_info("Boundaries test passed.");
+	return true;
+}
+
+static bool test_pattern(void)
+{
+	return (memchr_inv(array, '0', PAGE_SIZE / 2) ||
+		memchr_inv(array + PAGE_SIZE / 2, '1', PAGE_SIZE * 3 / 4) ||
+		memchr_inv(array + PAGE_SIZE * 5 / 4, '0', PAGE_SIZE / 2) ||
+		memchr_inv(array + PAGE_SIZE * 7 / 4, '1', PAGE_SIZE * 3 / 4) ||
+		memchr_inv(array + PAGE_SIZE * 5 / 2, '0', PAGE_SIZE / 2));
+}
+
+static bool test_wr_memset(void)
+{
+	int new_val = '1';
+
+	wr_memset(&scalar, new_val, sizeof(scalar));
+	if (WARN(memchr_inv(&scalar, new_val, sizeof(scalar)),
+		 "Scalar write rare memset test failed."))
+		return false;
+
+	pr_info("Scalar write rare memset test passed.");
+
+	wr_memset(array, '0', PAGE_SIZE * 3);
+	if (WARN(memchr_inv(array, '0', PAGE_SIZE * 3),
+		 "Array write rare memset test failed."))
+		return false;
+
+	wr_memset(array + PAGE_SIZE / 2, '1', PAGE_SIZE * 2);
+	if (WARN(memchr_inv(array + PAGE_SIZE / 2, '1', PAGE_SIZE * 2),
+		 "Array write rare memset test failed."))
+		return false;
+
+	wr_memset(array + PAGE_SIZE * 5 / 4, '0', PAGE_SIZE / 2);
+	if (WARN(memchr_inv(array + PAGE_SIZE * 5 / 4, '0', PAGE_SIZE / 2),
+		 "Array write rare memset test failed."))
+		return false;
+
+	if (WARN(test_pattern(), "Array write rare memset test failed."))
+		return false;
+
+	pr_info("Array write rare memset test passed.");
+	return true;
+}
+
+static u8 array_1[PAGE_SIZE * 2];
+static u8 array_2[PAGE_SIZE * 2];
+
+static bool test_wr_memcpy(void)
+{
+	int new_val = 0x12345678;
+
+	wr_assign(scalar, new_val);
+	if (WARN(memcmp(&scalar, &new_val, sizeof(scalar)),
+		 "Scalar write rare memcpy test failed."))
+		return false;
+	pr_info("Scalar write rare memcpy test passed.");
+
+	wr_memset(array, '0', PAGE_SIZE * 3);
+	memset(array_1, '1', PAGE_SIZE * 2);
+	memset(array_2, '0', PAGE_SIZE * 2);
+	wr_memcpy(array + PAGE_SIZE / 2, array_1, PAGE_SIZE * 2);
+	wr_memcpy(array + PAGE_SIZE * 5 / 4, array_2, PAGE_SIZE / 2);
+
+	if (WARN(test_pattern(), "Array write rare memcpy test failed."))
+		return false;
+
+	pr_info("Array write rare memcpy test passed.");
+	return true;
+}
+
+static __wr_after_init int *dst;
+static int reference = 0x54;
+
+static bool test_wr_rcu_assign_pointer(void)
+{
+	wr_rcu_assign_pointer(dst, &reference);
+	return dst == &reference;
+}
+
+static int __init test_static_wr_init_module(void)
+{
+	pr_info("static write rare test");
+	if (WARN(!(test_alignment() &&
+		   test_wr_memset() &&
+		   test_wr_memcpy() &&
+		   test_wr_rcu_assign_pointer()),
+		 "static write rare test failed"))
+		return -EFAULT;
+	pr_info("static write rare test passed");
+	return 0;
+}
+
+module_init(test_static_wr_init_module);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Igor Stoppa <igor.stoppa@huawei.com>");
+MODULE_DESCRIPTION("Test module for static write rare.");
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v4 12/12] IMA: turn ima_policy_flags into __wr_after_init
  2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
                   ` (10 preceding siblings ...)
  2019-02-11 23:27 ` [RFC PATCH v4 11/12] __wr_after_init: test write rare functionality Igor Stoppa
@ 2019-02-11 23:27 ` Igor Stoppa
  2019-02-12  0:09 ` [RFC PATCH v4 00/12] hardening: statically allocated protected memory Kees Cook
  12 siblings, 0 replies; 22+ messages in thread
From: Igor Stoppa @ 2019-02-11 23:27 UTC (permalink / raw)
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Matthew Wilcox,
	Peter Zijlstra, Kees Cook, Dave Hansen, Mimi Zohar,
	Thiago Jung Bauermann, Ahmed Soliman, linux-integrity,
	kernel-hardening, linux-mm, linux-kernel

The policy flags could be targeted by an attacker aiming at disabling IMA,
so that there would be no trace of a file system modification in the
measurement list.

Since the flags can be altered at runtime, it is not possible to make
them become fully read-only, for example with __ro_after_init.

__wr_after_init can still provide some protection, at least against
simple memory overwrite attacks

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>

CC: Andy Lutomirski <luto@amacapital.net>
CC: Nadav Amit <nadav.amit@gmail.com>
CC: Matthew Wilcox <willy@infradead.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Kees Cook <keescook@chromium.org>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Mimi Zohar <zohar@linux.vnet.ibm.com>
CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Ahmed Soliman <ahmedsoliman@mena.vt.edu>
CC: linux-integrity@vger.kernel.org
CC: kernel-hardening@lists.openwall.com
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
---
 security/integrity/ima/ima.h        | 3 ++-
 security/integrity/ima/ima_policy.c | 9 +++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index cc12f3449a72..297c25f5122e 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -24,6 +24,7 @@
 #include <linux/hash.h>
 #include <linux/tpm.h>
 #include <linux/audit.h>
+#include <linux/prmem.h>
 #include <crypto/hash_info.h>
 
 #include "../integrity.h"
@@ -50,7 +51,7 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
 #define IMA_TEMPLATE_IMA_FMT "d|n"
 
 /* current content of the policy */
-extern int ima_policy_flag;
+extern int ima_policy_flag __wr_after_init;
 
 /* set during initialization */
 extern int ima_hash_algo;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 8bc8a1c8cb3f..d49c545b9cfb 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -48,7 +48,7 @@
 #define INVALID_PCR(a) (((a) < 0) || \
 	(a) >= (FIELD_SIZEOF(struct integrity_iint_cache, measured_pcrs) * 8))
 
-int ima_policy_flag;
+int ima_policy_flag __wr_after_init;
 static int temp_ima_appraise;
 static int build_ima_appraise __ro_after_init;
 
@@ -460,12 +460,13 @@ void ima_update_policy_flag(void)
 
 	list_for_each_entry(entry, ima_rules, list) {
 		if (entry->action & IMA_DO_MASK)
-			ima_policy_flag |= entry->action;
+			wr_assign(ima_policy_flag,
+				  ima_policy_flag | entry->action);
 	}
 
 	ima_appraise |= (build_ima_appraise | temp_ima_appraise);
 	if (!ima_appraise)
-		ima_policy_flag &= ~IMA_APPRAISE;
+		wr_assign(ima_policy_flag, ima_policy_flag & ~IMA_APPRAISE);
 }
 
 static int ima_appraise_flag(enum ima_hooks func)
@@ -651,7 +652,7 @@ void ima_update_policy(void)
 	list_splice_tail_init_rcu(&ima_temp_rules, policy, synchronize_rcu);
 
 	if (ima_rules != policy) {
-		ima_policy_flag = 0;
+		wr_assign(ima_policy_flag, 0);
 		ima_rules = policy;
 
 		/*
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH v4 00/12] hardening: statically allocated protected memory
  2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
                   ` (11 preceding siblings ...)
  2019-02-11 23:27 ` [RFC PATCH v4 12/12] IMA: turn ima_policy_flags into __wr_after_init Igor Stoppa
@ 2019-02-12  0:09 ` Kees Cook
  2019-02-12  0:37   ` Igor Stoppa
  12 siblings, 1 reply; 22+ messages in thread
From: Kees Cook @ 2019-02-12  0:09 UTC (permalink / raw)
  To: Igor Stoppa
  Cc: Igor Stoppa, Ahmed Soliman, linux-integrity, Kernel Hardening,
	Linux-MM, Linux Kernel Mailing List

On Mon, Feb 11, 2019 at 3:28 PM Igor Stoppa <igor.stoppa@gmail.com> wrote:
> at last I'm able to resume work on the memory protection patchset I've
> proposed some time ago. This version should address comments received so
> far and introduce support for arm64. Details below.

Cool!

> Patch-set implementing write-rare memory protection for statically
> allocated data.

It seems like this could be expanded in the future to cover dynamic
memory too (i.e. just a separate base range in the mm).

> Its purpose is to keep write protected the kernel data which is seldom
> modified, especially if altering it can be exploited during an attack.
>
> There is no read overhead, however writing requires special operations that
> are probably unsuitable for often-changing data.
> The use is opt-in, by applying the modifier __wr_after_init to a variable
> declaration.
>
> As the name implies, the write protection kicks in only after init() is
> completed; before that moment, the data is modifiable in the usual way.
>
> Current Limitations:
> * supports only data which is allocated statically, at build time.
> * supports only x86_64 and arm64;other architectures need to provide own
>   backend

It looked like only the memset() needed architecture support. Is there
a reason for not being able to implement memset() in terms of an
inefficient put_user() loop instead? That would eliminate the need for
per-arch support, yes?

> - I've added a simple example: the protection of ima_policy_flags

You'd also looked at SELinux too, yes? What other things could be
targeted for protection? (It seems we can't yet protect page tables
themselves with this...)

> - the x86_64 user space address range is double the size of the kernel
>   address space, so it's possible to randomize the beginning of the
>   mapping of the kernel address space, but on arm64 they have the same
>   size, so it's not possible to do the same

Only the wr_rare section needs mapping, though, yes?

> - I'm not sure if it's correct, since it doesn't seem to be that common in
>   kernel sources, but instead of using #defines for overriding default
>   function calls, I'm using "weak" for the default functions.

The tradition is to use #defines for easier readability, but "weak"
continues to be a thing. *shrug*

This will be a nice addition to protect more of the kernel's static
data from write-what-where attacks. :)

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH v4 00/12] hardening: statically allocated protected memory
  2019-02-12  0:09 ` [RFC PATCH v4 00/12] hardening: statically allocated protected memory Kees Cook
@ 2019-02-12  0:37   ` Igor Stoppa
  2019-02-12  0:46     ` Kees Cook
  0 siblings, 1 reply; 22+ messages in thread
From: Igor Stoppa @ 2019-02-12  0:37 UTC (permalink / raw)
  To: Kees Cook
  Cc: Igor Stoppa, Ahmed Soliman, linux-integrity, Kernel Hardening,
	Linux-MM, Linux Kernel Mailing List



On 12/02/2019 02:09, Kees Cook wrote:
> On Mon, Feb 11, 2019 at 3:28 PM Igor Stoppa <igor.stoppa@gmail.com> wrote:

[...]

>> Patch-set implementing write-rare memory protection for statically
>> allocated data.
> 
> It seems like this could be expanded in the future to cover dynamic
> memory too (i.e. just a separate base range in the mm).

Indeed. And part of the code refactoring is also geared in that 
direction. I am working on that part, but it was agreed that I would 
first provide this subset of features covering statically allocated 
memory. So I'm sticking to the plan. But this is roughly 1/3 of the 
basic infra I have in mind.

>> Its purpose is to keep write protected the kernel data which is seldom
>> modified, especially if altering it can be exploited during an attack.
>>
>> There is no read overhead, however writing requires special operations that
>> are probably unsuitable for often-changing data.
>> The use is opt-in, by applying the modifier __wr_after_init to a variable
>> declaration.
>>
>> As the name implies, the write protection kicks in only after init() is
>> completed; before that moment, the data is modifiable in the usual way.
>>
>> Current Limitations:
>> * supports only data which is allocated statically, at build time.
>> * supports only x86_64 and arm64;other architectures need to provide own
>>    backend
> 
> It looked like only the memset() needed architecture support. Is there
> a reason for not being able to implement memset() in terms of an
> inefficient put_user() loop instead? That would eliminate the need for
> per-arch support, yes?

So far, yes, however from previous discussion about power arch, I 
understood this implementation would not be so easy to adapt.
Lacking other examples where the extra mapping could be used, I did not 
want to add code without a use case.

Probably both arm and x86 32 bit could do, but I would like to first get 
to the bitter end with memory protection (the other 2 thirds).

Mostly, I hated having just one arch and I also really wanted to have arm64.

But eventually, yes, a generic put_user() loop could do, provided that 
there are other arch where the extra mapping to user space would be a 
good way to limit write access. This last part is what I'm not sure of.

>> - I've added a simple example: the protection of ima_policy_flags
> 
> You'd also looked at SELinux too, yes? What other things could be
> targeted for protection? (It seems we can't yet protect page tables
> themselves with this...)

Yes, I have. See the "1/3" explanation above. I'm also trying to get 
away with as small example as possible, to get the basic infra merged.
SELinux is not going to be a small patch set. I'd rather move to it once 
at least some of the framework is merged. It might be a good use case 
for dynamic allocation, if I do not find something smaller.
But for static write rare, going after IMA was easier, and it is still a 
good target for protection, imho, as flipping this variable should be 
sufficient for turning IMA off.

For the page tables, I have in mind a little bit different approach, 
that I hope to explain better once I get to do the dynamic allocation.

>> - the x86_64 user space address range is double the size of the kernel
>>    address space, so it's possible to randomize the beginning of the
>>    mapping of the kernel address space, but on arm64 they have the same
>>    size, so it's not possible to do the same
> 
> Only the wr_rare section needs mapping, though, yes?

Yup, however, once more, I'm not so keen to do what seems as premature 
optimization, before I have addressed the framework in its entirety, as 
the dynamic allocation will need similar treatment.

>> - I'm not sure if it's correct, since it doesn't seem to be that common in
>>    kernel sources, but instead of using #defines for overriding default
>>    function calls, I'm using "weak" for the default functions.
> 
> The tradition is to use #defines for easier readability, but "weak"
> continues to be a thing. *shrug*

Yes, I wasn't so sure about it, but I kinda liked the fact that, by 
using "weak", the arch header becomes optional, unless one has to 
redefine the struct wr_state.

> This will be a nice addition to protect more of the kernel's static
> data from write-what-where attacks. :)

I hope so :-)

--
thanks, igor

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH v4 00/12] hardening: statically allocated protected memory
  2019-02-12  0:37   ` Igor Stoppa
@ 2019-02-12  0:46     ` Kees Cook
  2019-02-12  1:08       ` igor.stoppa
  0 siblings, 1 reply; 22+ messages in thread
From: Kees Cook @ 2019-02-12  0:46 UTC (permalink / raw)
  To: Igor Stoppa
  Cc: Igor Stoppa, Ahmed Soliman, linux-integrity, Kernel Hardening,
	Linux-MM, Linux Kernel Mailing List

On Mon, Feb 11, 2019 at 4:37 PM Igor Stoppa <igor.stoppa@gmail.com> wrote:
>
>
>
> On 12/02/2019 02:09, Kees Cook wrote:
> > On Mon, Feb 11, 2019 at 3:28 PM Igor Stoppa <igor.stoppa@gmail.com> wrote:
> > It looked like only the memset() needed architecture support. Is there
> > a reason for not being able to implement memset() in terms of an
> > inefficient put_user() loop instead? That would eliminate the need for
> > per-arch support, yes?
>
> So far, yes, however from previous discussion about power arch, I
> understood this implementation would not be so easy to adapt.
> Lacking other examples where the extra mapping could be used, I did not
> want to add code without a use case.
>
> Probably both arm and x86 32 bit could do, but I would like to first get
> to the bitter end with memory protection (the other 2 thirds).
>
> Mostly, I hated having just one arch and I also really wanted to have arm64.

Right, I meant, if you implemented the _memset() case with put_user()
in this version, you could drop the arch-specific _memset() and shrink
the patch series. Then you could also enable this across all the
architectures in one patch. (Would you even need the Kconfig patches,
i.e. won't this "Just Work" on everything with an MMU?)

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH v4 00/12] hardening: statically allocated protected memory
  2019-02-12  0:46     ` Kees Cook
@ 2019-02-12  1:08       ` igor.stoppa
  2019-02-12  1:26         ` Kees Cook
  0 siblings, 1 reply; 22+ messages in thread
From: igor.stoppa @ 2019-02-12  1:08 UTC (permalink / raw)
  To: Kees Cook
  Cc: Igor Stoppa, Ahmed Soliman, linux-integrity, Kernel Hardening,
	Linux-MM, Linux Kernel Mailing List

[-- Attachment #1: Type: text/plain, Size: 1608 bytes --]

On Tue, 12 Feb 2019, 4.47 Kees Cook <keescook@chromium.org wrote:

> On Mon, Feb 11, 2019 at 4:37 PM Igor Stoppa <igor.stoppa@gmail.com> wrote:
> >
> >
> >
> > On 12/02/2019 02:09, Kees Cook wrote:
> > > On Mon, Feb 11, 2019 at 3:28 PM Igor Stoppa <igor.stoppa@gmail.com>
> wrote:
> > > It looked like only the memset() needed architecture support. Is there
> > > a reason for not being able to implement memset() in terms of an
> > > inefficient put_user() loop instead? That would eliminate the need for
> > > per-arch support, yes?
> >
> > So far, yes, however from previous discussion about power arch, I
> > understood this implementation would not be so easy to adapt.
> > Lacking other examples where the extra mapping could be used, I did not
> > want to add code without a use case.
> >
> > Probably both arm and x86 32 bit could do, but I would like to first get
> > to the bitter end with memory protection (the other 2 thirds).
> >
> > Mostly, I hated having just one arch and I also really wanted to have
> arm64.
>
> Right, I meant, if you implemented the _memset() case with put_user()
> in this version, you could drop the arch-specific _memset() and shrink
> the patch series. Then you could also enable this across all the
> architectures in one patch. (Would you even need the Kconfig patches,
> i.e. won't this "Just Work" on everything with an MMU?)
>

I had similar thoughts, but this answer [1] deflated my hopes (if I
understood it correctly).
It seems that each arch needs to be massaged in separately.

--
igor


[1] https://www.openwall.com/lists/kernel-hardening/2018/12/12/15

>

[-- Attachment #2: Type: text/html, Size: 2638 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH v4 00/12] hardening: statically allocated protected memory
  2019-02-12  1:08       ` igor.stoppa
@ 2019-02-12  1:26         ` Kees Cook
  2019-02-12  7:09           ` Igor Stoppa
  0 siblings, 1 reply; 22+ messages in thread
From: Kees Cook @ 2019-02-12  1:26 UTC (permalink / raw)
  To: igor.stoppa
  Cc: Igor Stoppa, Ahmed Soliman, linux-integrity, Kernel Hardening,
	Linux-MM, Linux Kernel Mailing List

On Mon, Feb 11, 2019 at 5:08 PM igor.stoppa@gmail.com
<igor.stoppa@gmail.com> wrote:
>
>
>
> On Tue, 12 Feb 2019, 4.47 Kees Cook <keescook@chromium.org wrote:
>>
>> On Mon, Feb 11, 2019 at 4:37 PM Igor Stoppa <igor.stoppa@gmail.com> wrote:
>> >
>> >
>> >
>> > On 12/02/2019 02:09, Kees Cook wrote:
>> > > On Mon, Feb 11, 2019 at 3:28 PM Igor Stoppa <igor.stoppa@gmail.com> wrote:
>> > > It looked like only the memset() needed architecture support. Is there
>> > > a reason for not being able to implement memset() in terms of an
>> > > inefficient put_user() loop instead? That would eliminate the need for
>> > > per-arch support, yes?
>> >
>> > So far, yes, however from previous discussion about power arch, I
>> > understood this implementation would not be so easy to adapt.
>> > Lacking other examples where the extra mapping could be used, I did not
>> > want to add code without a use case.
>> >
>> > Probably both arm and x86 32 bit could do, but I would like to first get
>> > to the bitter end with memory protection (the other 2 thirds).
>> >
>> > Mostly, I hated having just one arch and I also really wanted to have arm64.
>>
>> Right, I meant, if you implemented the _memset() case with put_user()
>> in this version, you could drop the arch-specific _memset() and shrink
>> the patch series. Then you could also enable this across all the
>> architectures in one patch. (Would you even need the Kconfig patches,
>> i.e. won't this "Just Work" on everything with an MMU?)
>
>
> I had similar thoughts, but this answer [1] deflated my hopes (if I understood it correctly).
> It seems that each arch needs to be massaged in separately.

True, but I think x86_64, x86, arm64, and arm will all be "normal".
power may be that way too, but they always surprise me. :)

Anyway, series looks good, but since nothing uses _memset(), it might
make sense to leave it out and put all the arch-enabling into a single
patch to cover the 4 archs above, in an effort to make the series even
smaller.

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH v4 01/12] __wr_after_init: Core and default arch
  2019-02-11 23:27 ` [RFC PATCH v4 01/12] __wr_after_init: Core and default arch Igor Stoppa
@ 2019-02-12  2:39   ` Matthew Wilcox
  2019-02-12  7:20     ` Igor Stoppa
  0 siblings, 1 reply; 22+ messages in thread
From: Matthew Wilcox @ 2019-02-12  2:39 UTC (permalink / raw)
  To: Igor Stoppa
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Peter Zijlstra,
	Kees Cook, Dave Hansen, Mimi Zohar, Thiago Jung Bauermann,
	Ahmed Soliman, linux-integrity, kernel-hardening, linux-mm,
	linux-kernel

On Tue, Feb 12, 2019 at 01:27:38AM +0200, Igor Stoppa wrote:
> +#ifndef CONFIG_PRMEM
[...]
> +#else
> +
> +#include <linux/mm.h>

It's a mistake to do conditional includes like this.  That way you see
include loops with some configs and not others.  Our headers are already
so messy, better to just include mm.h unconditionally.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH v4 00/12] hardening: statically allocated protected memory
  2019-02-12  1:26         ` Kees Cook
@ 2019-02-12  7:09           ` Igor Stoppa
  2019-02-12 22:39             ` Kees Cook
  0 siblings, 1 reply; 22+ messages in thread
From: Igor Stoppa @ 2019-02-12  7:09 UTC (permalink / raw)
  To: Kees Cook
  Cc: Igor Stoppa, Ahmed Soliman, linux-integrity, Kernel Hardening,
	Linux-MM, Linux Kernel Mailing List



On 12/02/2019 03:26, Kees Cook wrote:
> On Mon, Feb 11, 2019 at 5:08 PM igor.stoppa@gmail.com
> <igor.stoppa@gmail.com> wrote:
>>
>>
>>
>> On Tue, 12 Feb 2019, 4.47 Kees Cook <keescook@chromium.org wrote:
>>>
>>> On Mon, Feb 11, 2019 at 4:37 PM Igor Stoppa <igor.stoppa@gmail.com> wrote:
>>>>
>>>>
>>>>
>>>> On 12/02/2019 02:09, Kees Cook wrote:
>>>>> On Mon, Feb 11, 2019 at 3:28 PM Igor Stoppa <igor.stoppa@gmail.com> wrote:
>>>>> It looked like only the memset() needed architecture support. Is there
>>>>> a reason for not being able to implement memset() in terms of an
>>>>> inefficient put_user() loop instead? That would eliminate the need for
>>>>> per-arch support, yes?
>>>>
>>>> So far, yes, however from previous discussion about power arch, I
>>>> understood this implementation would not be so easy to adapt.
>>>> Lacking other examples where the extra mapping could be used, I did not
>>>> want to add code without a use case.
>>>>
>>>> Probably both arm and x86 32 bit could do, but I would like to first get
>>>> to the bitter end with memory protection (the other 2 thirds).
>>>>
>>>> Mostly, I hated having just one arch and I also really wanted to have arm64.
>>>
>>> Right, I meant, if you implemented the _memset() case with put_user()
>>> in this version, you could drop the arch-specific _memset() and shrink
>>> the patch series. Then you could also enable this across all the
>>> architectures in one patch. (Would you even need the Kconfig patches,
>>> i.e. won't this "Just Work" on everything with an MMU?)
>>
>>
>> I had similar thoughts, but this answer [1] deflated my hopes (if I understood it correctly).
>> It seems that each arch needs to be massaged in separately.
> 
> True, but I think x86_64, x86, arm64, and arm will all be "normal".
> power may be that way too, but they always surprise me. :)
> 
> Anyway, series looks good, but since nothing uses _memset(), it might
> make sense to leave it out and put all the arch-enabling into a single
> patch to cover the 4 archs above, in an effort to make the series even
> smaller.

Actually, I do use it, albeit indirectly.
That's the whole point of having the IMA patch as example.

This is the fragment:
----------------
@@ -460,12 +460,13 @@ void ima_update_policy_flag(void)

  	list_for_each_entry(entry, ima_rules, list) {
  		if (entry->action & IMA_DO_MASK)
-			ima_policy_flag |= entry->action;
+			wr_assign(ima_policy_flag,
+				  ima_policy_flag | entry->action);
  	}

  	ima_appraise |= (build_ima_appraise | temp_ima_appraise);
  	if (!ima_appraise)
-		ima_policy_flag &= ~IMA_APPRAISE;
+		wr_assign(ima_policy_flag, ima_policy_flag & ~IMA_APPRAISE);
  }
----------------

wr_assign() does just that.

However, reading again your previous mails, I realize that I might have 
misinterpreted what you were suggesting.

If the advice is to have also a default memset_user() which relies on 
put_user(), but do not activate the feature by default for every 
architecture, I definitely agree that it would be good to have it.
I just didn't think about it before.

What I cannot do is to turn it on for all the architectures prior to 
test it and atm I do not have means to do it.

But I now realize that most likely you were just suggesting to have 
full, albeit inefficient default support and then let various archs 
review/enhance it. I can certainly do this.

Regarding testing I have a question: how much can/should I lean on qemu?
In most cases the MMU might not need to be fully emulated, so I wonder 
how well qemu-based testing can ensure that real life scenarios will work.

--
igor

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH v4 01/12] __wr_after_init: Core and default arch
  2019-02-12  2:39   ` Matthew Wilcox
@ 2019-02-12  7:20     ` Igor Stoppa
  0 siblings, 0 replies; 22+ messages in thread
From: Igor Stoppa @ 2019-02-12  7:20 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Igor Stoppa, Andy Lutomirski, Nadav Amit, Peter Zijlstra,
	Kees Cook, Dave Hansen, Mimi Zohar, Thiago Jung Bauermann,
	Ahmed Soliman, linux-integrity, kernel-hardening, linux-mm,
	linux-kernel



On 12/02/2019 04:39, Matthew Wilcox wrote:
> On Tue, Feb 12, 2019 at 01:27:38AM +0200, Igor Stoppa wrote:
>> +#ifndef CONFIG_PRMEM
> [...]
>> +#else
>> +
>> +#include <linux/mm.h>
> 
> It's a mistake to do conditional includes like this.  That way you see
> include loops with some configs and not others.  Our headers are already
> so messy, better to just include mm.h unconditionally.
> 

ok

Can I still do the following, in prmem.c ?

#ifdef CONFIG_ARCH_HAS_PRMEM_HEADER
+#include <asm/prmem.h>
+#else
+
+struct wr_state {
+       struct mm_struct *prev;
+};
+
+#endif


It's still a conditional include, but it's in a C file, it shouldn't 
cause any chance of loops.

The alternative is that each arch supporting prmem must have a 
(probably) empty asm/prmem.h header.

I did some reasearch about telling the compiler to include a header only 
if it exists, but it doesn't seem to be a gcc feature.

--
igor

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH v4 00/12] hardening: statically allocated protected memory
  2019-02-12  7:09           ` Igor Stoppa
@ 2019-02-12 22:39             ` Kees Cook
  0 siblings, 0 replies; 22+ messages in thread
From: Kees Cook @ 2019-02-12 22:39 UTC (permalink / raw)
  To: Igor Stoppa
  Cc: Igor Stoppa, Ahmed Soliman, linux-integrity, Kernel Hardening,
	Linux-MM, Linux Kernel Mailing List

On Mon, Feb 11, 2019 at 11:09 PM Igor Stoppa <igor.stoppa@gmail.com> wrote:
> wr_assign() does just that.
>
> However, reading again your previous mails, I realize that I might have
> misinterpreted what you were suggesting.
>
> If the advice is to have also a default memset_user() which relies on
> put_user(), but do not activate the feature by default for every
> architecture, I definitely agree that it would be good to have it.
> I just didn't think about it before.

Yeah, I just mean you could have an arch-agnostic memset_user() implementation.

> But I now realize that most likely you were just suggesting to have
> full, albeit inefficient default support and then let various archs
> review/enhance it. I can certainly do this.

Right.

> Regarding testing I have a question: how much can/should I lean on qemu?
> In most cases the MMU might not need to be fully emulated, so I wonder
> how well qemu-based testing can ensure that real life scenarios will work.

I think qemu lets you know if it works (kvm is using the real MMU),
and baremetal will give you more stable performance numbers.

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2019-02-12 22:39 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-11 23:27 [RFC PATCH v4 00/12] hardening: statically allocated protected memory Igor Stoppa
2019-02-11 23:27 ` [RFC PATCH v4 01/12] __wr_after_init: Core and default arch Igor Stoppa
2019-02-12  2:39   ` Matthew Wilcox
2019-02-12  7:20     ` Igor Stoppa
2019-02-11 23:27 ` [RFC PATCH v4 02/12] __wr_after_init: x86_64: memset_user() Igor Stoppa
2019-02-11 23:27 ` [RFC PATCH v4 03/12] __wr_after_init: x86_64: randomize mapping offset Igor Stoppa
2019-02-11 23:27 ` [RFC PATCH v4 04/12] __wr_after_init: x86_64: enable Igor Stoppa
2019-02-11 23:27 ` [RFC PATCH v4 05/12] __wr_after_init: arm64: memset_user() Igor Stoppa
2019-02-11 23:27 ` [RFC PATCH v4 06/12] __wr_after_init: arm64: enable Igor Stoppa
2019-02-11 23:27 ` [RFC PATCH v4 07/12] __wr_after_init: Documentation: self-protection Igor Stoppa
2019-02-11 23:27 ` [RFC PATCH v4 08/12] __wr_after_init: lkdtm test Igor Stoppa
2019-02-11 23:27 ` [RFC PATCH v4 09/12] __wr_after_init: rodata_test: refactor tests Igor Stoppa
2019-02-11 23:27 ` [RFC PATCH v4 10/12] __wr_after_init: rodata_test: test __wr_after_init Igor Stoppa
2019-02-11 23:27 ` [RFC PATCH v4 11/12] __wr_after_init: test write rare functionality Igor Stoppa
2019-02-11 23:27 ` [RFC PATCH v4 12/12] IMA: turn ima_policy_flags into __wr_after_init Igor Stoppa
2019-02-12  0:09 ` [RFC PATCH v4 00/12] hardening: statically allocated protected memory Kees Cook
2019-02-12  0:37   ` Igor Stoppa
2019-02-12  0:46     ` Kees Cook
2019-02-12  1:08       ` igor.stoppa
2019-02-12  1:26         ` Kees Cook
2019-02-12  7:09           ` Igor Stoppa
2019-02-12 22:39             ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).