Skip to contentSkip to conversation

hardening: statically allocated protected memory

RFC PATCH In review Igor Stoppa 2 participants 16 messages Feb 13, 2019 – Feb 16, 2019

Subsystems (14)
lore
Share & tools
lore
#0 lore raw

v5 cover hardening: statically allocated protected memory

To: Andy Lutomirski <[email protected]>,
To: Matthew Wilcox <[email protected]>,
To: Nadav Amit <[email protected]>
To: Peter Zijlstra <[email protected]>,
To: Dave Hansen <[email protected]>,
To: Mimi Zohar <[email protected]>
To: Thiago Jung Bauermann <[email protected]>
CC: Kees Cook <[email protected]>
CC: Ahmed Soliman <[email protected]>
CC: linux-integrity <[email protected]>
CC: Kernel Hardening <[email protected]>
CC: Linux-MM <[email protected]>
CC: Linux Kernel Mailing List <[email protected]>

Hello,
new version of the patchset, with default memset_user() function.

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.
* verified (and enabled) only x86_64 and arm64; other architectures need to be tested, possibly providing 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 verified, 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. Eventually, the randomization could affect exclusively the ranges containing protectable memory, but this should be done togeter with the protection of dynamically allocated data (once it is available).
- unaddressed: Nadav proposed to do:

	#define __wr          __attribute__((address_space(5)))

but I don't know exactly where to use it atm

Changelog:

v4->v5
------
* turned conditional inclusion of mm.h into permanent
* added generic, albeit unoptimized memset_user() function
* more verbose error messages for testing of wr_memset()

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 (11):
__wr_after_init: linker section and attribute __wr_after_init: Core and default arch __wr_after_init: x86_64: randomize mapping offset __wr_after_init: x86_64: enable __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

Nadav Amit (1):
fork: provide a function for copying init_mm

 Documentation/security/self-protection.rst |  14 +-
 arch/Kconfig                               |  22 +++
 arch/arm64/Kconfig                         |   1 +
 arch/x86/Kconfig                           |   1 +
 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/asm-generic/vmlinux.lds.h          |  25 +++
 include/linux/cache.h                      |  21 +++
 include/linux/prmem.h (new)                |  70 ++++++++
 include/linux/sched/task.h                 |   1 +
 init/main.c                                |   3 +
 kernel/fork.c                              |  24 ++-
 mm/Kconfig.debug                           |   8 +
 mm/Makefile                                |   2 +
 mm/prmem.c (new)                           | 193 +++++++++++++++++++++
 mm/rodata_test.c                           |  69 +++++---
 mm/test_write_rare.c (new)                 | 142 +++++++++++++++
 security/integrity/ima/ima.h               |   3 +-
 security/integrity/ima/ima_policy.c        |   9 +-
 22 files changed, 628 insertions(+), 37 deletions(-)
 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
Signature · 1 line
2.19.1
#1 lore raw

RFC PATCH v5 2/12 __wr_after_init: linker section and attribute

Introduce a linker section and a matching attribute for statically allocated write rare data. The attribute is named "__wr_after_init". After the init phase is completed, this section will be modifiable only by invoking write rare functions.
The section occupies a set of full pages, since the granularity available for write protection is of one memory page.

The functionality is automatically activated by any architecture that sets CONFIG_ARCH_HAS_PRMEM

CC Andy Lutomirski <[email protected]>
CC Nadav Amit <[email protected]>
CC Matthew Wilcox <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Kees Cook <[email protected]>
CC Dave Hansen <[email protected]>
CC Mimi Zohar <[email protected]>
CC Thiago Jung Bauermann <[email protected]>
CC Ahmed Soliman <[email protected]>
---
 arch/Kconfig                      | 15 +++++++++++++++
 include/asm-generic/vmlinux.lds.h | 25 +++++++++++++++++++++++++
 include/linux/cache.h             | 21 +++++++++++++++++++++
 init/main.c                       |  3 +++
 4 files changed, 64 insertions(+)
arch/Kconfig +15 -0
index 4cfb6de48f79..b0b6d176f1c1 100644
@@ -808,6 +808,21 @@ config VMAP_STACK
 	  the stack to map directly to the KASAN shadow map using a formula
 	  that is incorrect if the stack is in vmalloc space.
 
+config ARCH_HAS_PRMEM
+	def_bool n
+	help
+	  architecture specific symbol stating that the architecture provides
+	  a back-end function for the write rare operation.
+
+config PRMEM
+	bool "Write protect critical data that doesn't need high write speed."
+	depends on ARCH_HAS_PRMEM
+	default y
+	help
+	  If the architecture supports it, statically allocated data which
+	  has been selected for hardening becomes (mostly) read-only.
+	  The selection happens by labelling the data "__wr_after_init".
+
 config ARCH_OPTIONAL_KERNEL_RWX
 	def_bool n
 
include/asm-generic/vmlinux.lds.h +25 -0
index 3d7a6a9c2370..ddb1fd608490 100644
@@ -311,6 +311,30 @@
 	KEEP(*(__jump_table))						\
 	__stop___jump_table = .;
 
+/*
+ * Allow architectures to handle wr_after_init data on their
+ * own by defining an empty WR_AFTER_INIT_DATA.
+ * However, it's important that pages containing WR_RARE data do not
+ * hold anything else, to avoid both accidentally unprotecting something
+ * that is supposed to stay read-only all the time and also to protect
+ * something else that is supposed to be writeable all the time.
+ */
+#ifndef WR_AFTER_INIT_DATA
+#ifdef CONFIG_PRMEM
+#define WR_AFTER_INIT_DATA(align)					\
+	. = ALIGN(PAGE_SIZE);						\
+	__start_wr_after_init = .;					\
+	. = ALIGN(align);						\
+	*(.data..wr_after_init)						\
+	. = ALIGN(PAGE_SIZE);						\
+	__end_wr_after_init = .;					\
+	. = ALIGN(align);
+#else
+#define WR_AFTER_INIT_DATA(align)					\
+	. = ALIGN(align);
+#endif
+#endif
+
 /*
  * Allow architectures to handle ro_after_init data on their
  * own by defining an empty RO_AFTER_INIT_DATA.
@@ -332,6 +356,7 @@
 		__start_rodata = .;					\
 		*(.rodata) *(.rodata.*)					\
 		RO_AFTER_INIT_DATA	/* Read only after init */	\
+		WR_AFTER_INIT_DATA(align) /* wr after init */	\
 		KEEP(*(__vermagic))	/* Kernel version magic */	\
 		. = ALIGN(8);						\
 		__start___tracepoints_ptrs = .;				\
include/linux/cache.h +21 -0
index 750621e41d1c..09bd0b9284b6 100644
@@ -31,6 +31,27 @@
 #define __ro_after_init __attribute__((__section__(".data..ro_after_init")))
 #endif
 
+/*
+ * __wr_after_init is used to mark objects that cannot be modified
+ * directly after init (i.e. after mark_rodata_ro() has been called).
+ * These objects become effectively read-only, from the perspective of
+ * performing a direct write, like a variable assignment.
+ * However, they can be altered through a dedicated function.
+ * It is intended for those objects which are occasionally modified after
+ * init, however they are modified so seldomly, that the extra cost from
+ * the indirect modification is either negligible or worth paying, for the
+ * sake of the protection gained.
+ */
+#ifndef __wr_after_init
+#ifdef CONFIG_PRMEM
+#define __wr_after_init \
+		__attribute__((__section__(".data..wr_after_init")))
+#else
+#define __wr_after_init
+#endif
+#endif
+
+
 #ifndef ____cacheline_aligned
 #define ____cacheline_aligned __attribute__((__aligned__(SMP_CACHE_BYTES)))
 #endif
init/main.c +3 -0
index c86a1c8f19f4..965e9fbc5452 100644
@@ -496,6 +496,8 @@ void __init __weak thread_stack_cache_init(void)
 
 void __init __weak mem_encrypt_init(void) { }
 
+void __init __weak wr_init(void) { }
+
 bool initcall_debug;
 core_param(initcall_debug, initcall_debug, bool, 0644);
 
@@ -713,6 +715,7 @@ asmlinkage __visible void __init start_kernel(void)
 	cred_init();
 	fork_init();
 	proc_caches_init();
+	wr_init();
 	uts_ns_init();
 	buffer_init();
 	key_init();
Signature · 1 line
2.19.1
#2 lore raw

RFC PATCH v5 3/12 __wr_after_init: Core and default arch

In reply to Igor Stoppa

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.

CC Andy Lutomirski <[email protected]>
CC Nadav Amit <[email protected]>
CC Matthew Wilcox <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Kees Cook <[email protected]>
CC Dave Hansen <[email protected]>
CC Mimi Zohar <[email protected]>
CC Thiago Jung Bauermann <[email protected]>
CC Ahmed Soliman <[email protected]>
---
 arch/Kconfig                |   7 ++
 include/linux/prmem.h (new) |  70 ++++++++++++++
 mm/Makefile                 |   1 +
 mm/prmem.c (new)            | 193 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 271 insertions(+)
arch/Kconfig +7 -0
index b0b6d176f1c1..0380d4a64681 100644
@@ -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
include/linux/prmem.h new file +70 -0

70 lines not on this page · raw message

mm/Makefile +1 -0
index d210cc9d6f80..ef3867c16ce0 100644
@@ -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
mm/prmem.c new file +193 -0

193 lines not on this page · raw message

Signature · 1 line
2.19.1
maintainer
#3 lore raw

Re: [RFC PATCH v5 03/12] __wr_after_init: Core and default arch

Quoted 41 lines · Igor Stoppa
include/linux/prmem.h new file +41 -0
+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
+
+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()

include/linux/prmem.h line 68

Quoted 10 lines
include/linux/prmem.h new file +10 -0
+ * 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;				\
+})

This requires that wr_memcpy() (through wr_assign) is single-copy-atomic for native types. There is not a comment in sight that states this.

Also, is this true of x86/arm64 memcpy ?

#4 lore raw

Re: [RFC PATCH v5 03/12] __wr_after_init: Core and default arch

Quoted 1 line · Peter Zijlstra

On Thu, Feb 14, 2019 at 12:41:32AM +0200, Igor Stoppa wrote:

[...]

Quoted 8 lines

include/linux/prmem.h line 68

Quoted 5 lines
include/linux/prmem.h new file +5 -0
+#define wr_rcu_assign_pointer(p, v) ({	\
+	smp_mb();			\
+	wr_assign(p, v);		\
+	p;				\
+})

This requires that wr_memcpy() (through wr_assign) is single-copy-atomic for native types. There is not a comment in sight that states this.

Right, I kinda expected native-aligned <-> atomic, but it's not necessarily true. It should be confirmed when enabling write rare on a new architecture. I'll add the comment.

Quoted 1 line

Also, is this true of x86/arm64 memcpy ?

For x86_64:
https://elixir.bootlin.com/linux/v5.0-rc6/source/arch/x86/include/asm/uaccess.h#L462
the mov"itype" part should deal with atomic copy of native, aligned
types.

For arm64:
https://elixir.bootlin.com/linux/v5.0-rc6/source/arch/arm64/lib/copy_template.S#L110 .Ltiny15 deals with copying less than 16 bytes, which includes pointers. When the data is aligned, the copy of a pointer should be atomic.

--
igor

maintainer
#5 lore raw

Re: [RFC PATCH v5 03/12] __wr_after_init: Core and default arch

Quoted 28 lines · Igor Stoppa
Quoted 1 line · Peter Zijlstra

On Thu, Feb 14, 2019 at 12:41:32AM +0200, Igor Stoppa wrote:

[...]

Quoted 8 lines

include/linux/prmem.h line 68

Quoted 5 lines
include/linux/prmem.h new file +5 -0
+#define wr_rcu_assign_pointer(p, v) ({	\
+	smp_mb();			\
+	wr_assign(p, v);		\
+	p;				\
+})

This requires that wr_memcpy() (through wr_assign) is single-copy-atomic for native types. There is not a comment in sight that states this.

Right, I kinda expected native-aligned <-> atomic, but it's not necessarily true. It should be confirmed when enabling write rare on a new architecture. I'll add the comment.

Quoted 1 line

Also, is this true of x86/arm64 memcpy ?

For x86_64:
https://elixir.bootlin.com/linux/v5.0-rc6/source/arch/x86/include/asm/uaccess.h#L462
the mov"itype" part should deal with atomic copy of native, aligned types.

Quoted 5 lines

For arm64:
https://elixir.bootlin.com/linux/v5.0-rc6/source/arch/arm64/lib/copy_template.S#L110
.Ltiny15 deals with copying less than 16 bytes, which includes pointers. When the data is aligned, the copy of a pointer should be atomic.

Where are the comments and Changelog notes ? How is an arch maintainer to be aware of this requirement when adding support for his/her arch?

#6 lore raw

Re: [RFC PATCH v5 03/12] __wr_after_init: Core and default arch

Quoted 2 lines · Peter Zijlstra

Where are the comments and Changelog notes ? How is an arch maintainer to be aware of this requirement when adding support for his/her arch?

Yes, it will be fixed in the next revision. I've added comment to the core wr_assign function and also to the changelogs for the patches enabling it on x86_64 and arm64, respectively.

Should I add mention of it also in the documentation?

--
igor

#7 lore raw

RFC PATCH v5 4/12 __wr_after_init: x86_64: randomize mapping offset

In reply to Igor Stoppa

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

CC Andy Lutomirski <[email protected]>
CC Nadav Amit <[email protected]>
CC Matthew Wilcox <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Kees Cook <[email protected]>
CC Dave Hansen <[email protected]>
CC Mimi Zohar <[email protected]>
CC Thiago Jung Bauermann <[email protected]>
CC Ahmed Soliman <[email protected]>
---
 arch/x86/mm/Makefile      |  2 ++
 arch/x86/mm/prmem.c (new) | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)
arch/x86/mm/Makefile +2 -0
index 4b101dd6e52f..66652de1e2c7 100644
@@ -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
arch/x86/mm/prmem.c new file +20 -0
new file mode 100644
index 000000000000..b04fc03f92fb
@@ -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 <[email protected]>
+ */
+
+#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));
+}
Signature · 1 line
2.19.1
#8 lore raw

RFC PATCH v5 5/12 __wr_after_init: x86_64: enable

In reply to Igor Stoppa

Set ARCH_HAS_PRMEM to Y for x86_64

CC Andy Lutomirski <[email protected]>
CC Nadav Amit <[email protected]>
CC Matthew Wilcox <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Kees Cook <[email protected]>
CC Dave Hansen <[email protected]>
CC Mimi Zohar <[email protected]>
CC Thiago Jung Bauermann <[email protected]>
CC Ahmed Soliman <[email protected]>
---
 arch/x86/Kconfig | 1 +
 1 file changed, 1 insertion(+)
arch/x86/Kconfig +1 -0
index 68261430fe6e..7392b53b12c2 100644
@@ -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
Signature · 1 line
2.19.1
#9 lore raw

RFC PATCH v5 6/12 __wr_after_init: arm64: enable

In reply to Igor Stoppa

Set ARCH_HAS_PRMEM to Y for arm64

CC Andy Lutomirski <[email protected]>
CC Nadav Amit <[email protected]>
CC Matthew Wilcox <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Kees Cook <[email protected]>
CC Dave Hansen <[email protected]>
CC Mimi Zohar <[email protected]>
CC Thiago Jung Bauermann <[email protected]>
CC Ahmed Soliman <[email protected]>
---
 arch/arm64/Kconfig | 1 +
 1 file changed, 1 insertion(+)
arch/arm64/Kconfig +1 -0
index a4168d366127..7cbb2c133ed7 100644
@@ -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
Signature · 1 line
2.19.1
#10 lore raw

RFC PATCH v5 7/12 __wr_after_init: Documentation: self-protection

In reply to Igor Stoppa

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

CC Andy Lutomirski <[email protected]>
CC Nadav Amit <[email protected]>
CC Matthew Wilcox <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Kees Cook <[email protected]>
CC Dave Hansen <[email protected]>
CC Mimi Zohar <[email protected]>
CC Thiago Jung Bauermann <[email protected]>
CC Ahmed Soliman <[email protected]>
---
 Documentation/security/self-protection.rst | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
Documentation/security/self-protection.rst +8 -6
index f584fb74b4ff..df2614bc25b9 100644
@@ -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
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signature · 1 line
2.19.1
#11 lore raw

RFC PATCH v5 8/12 __wr_after_init: lkdtm test

In reply to Igor Stoppa

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

CC Andy Lutomirski <[email protected]>
CC Nadav Amit <[email protected]>
CC Matthew Wilcox <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Kees Cook <[email protected]>
CC Dave Hansen <[email protected]>
CC Mimi Zohar <[email protected]>
CC Thiago Jung Bauermann <[email protected]>
CC Ahmed Soliman <[email protected]>
---
 drivers/misc/lkdtm/core.c  |  3 +++
 drivers/misc/lkdtm/lkdtm.h |  3 +++
 drivers/misc/lkdtm/perms.c | 29 +++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+)
drivers/misc/lkdtm/core.c +3 -0
index 2837dc77478e..73c34b17c433 100644
@@ -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),
drivers/misc/lkdtm/lkdtm.h +3 -0
index 3c6fd327e166..abba2f52ffa6 100644
@@ -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);
drivers/misc/lkdtm/perms.c +29 -0
index 53b85c9d16b8..f681730aa652 100644
@@ -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;
 }
Signature · 1 line
2.19.1
#12 lore raw

RFC PATCH v5 9/12 __wr_after_init: rodata_test: refactor tests

In reply to Igor Stoppa

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

CC Andy Lutomirski <[email protected]>
CC Nadav Amit <[email protected]>
CC Matthew Wilcox <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Kees Cook <[email protected]>
CC Dave Hansen <[email protected]>
CC Mimi Zohar <[email protected]>
CC Thiago Jung Bauermann <[email protected]>
CC Ahmed Soliman <[email protected]>
---
 mm/rodata_test.c | 48 ++++++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 20 deletions(-)
mm/rodata_test.c +28 -20
index d908c8769b48..e1349520b436 100644
@@ -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);
 }
Signature · 1 line
2.19.1
#13 lore raw

RFC PATCH v5 10/12 __wr_after_init: rodata_test: test __wr_after_init

In reply to Igor Stoppa

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

CC Andy Lutomirski <[email protected]>
CC Nadav Amit <[email protected]>
CC Matthew Wilcox <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Kees Cook <[email protected]>
CC Dave Hansen <[email protected]>
CC Mimi Zohar <[email protected]>
CC Thiago Jung Bauermann <[email protected]>
CC Ahmed Soliman <[email protected]>
---
 mm/rodata_test.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)
mm/rodata_test.c +24 -3
index e1349520b436..a669cf9f5a61 100644
@@ -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
 }
Signature · 1 line
2.19.1
#14 lore raw

RFC PATCH v5 11/12 __wr_after_init: test write rare functionality

In reply to Igor Stoppa

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

CC Andy Lutomirski <[email protected]>
CC Nadav Amit <[email protected]>
CC Matthew Wilcox <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Kees Cook <[email protected]>
CC Dave Hansen <[email protected]>
CC Mimi Zohar <[email protected]>
CC Thiago Jung Bauermann <[email protected]>
CC Ahmed Soliman <[email protected]>
---
 mm/Kconfig.debug           |   8 +++
 mm/Makefile                |   1 +
 mm/test_write_rare.c (new) | 142 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 151 insertions(+)
mm/Kconfig.debug +8 -0
index 9a7b8b049d04..a62c31901fea 100644
@@ -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.
mm/Makefile +1 -0
index ef3867c16ce0..8de1d468f4e7 100644
@@ -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
mm/test_write_rare.c new file +142 -0

142 lines not on this page · raw message

Signature · 1 line
2.19.1
#15 lore raw

RFC PATCH v5 12/12 IMA: turn ima_policy_flags into __wr_after_init

In reply to Igor Stoppa

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

CC Andy Lutomirski <[email protected]>
CC Nadav Amit <[email protected]>
CC Matthew Wilcox <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Kees Cook <[email protected]>
CC Dave Hansen <[email protected]>
CC Mimi Zohar <[email protected]>
CC Thiago Jung Bauermann <[email protected]>
CC Ahmed Soliman <[email protected]>
---
 security/integrity/ima/ima.h        | 3 ++-
 security/integrity/ima/ima_policy.c | 9 +++++----
 2 files changed, 7 insertions(+), 5 deletions(-)
security/integrity/ima/ima.h +2 -1
index cc12f3449a72..297c25f5122e 100644
@@ -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;
security/integrity/ima/ima_policy.c +5 -4
index 8bc8a1c8cb3f..d49c545b9cfb 100644
@@ -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;
 
 		/*
Signature · 1 line
2.19.1