[RFC] Stricter kernel memory permissions

[RFC] Stricter kernel memory permissions

[Laura Abbott]

This is an RFC to add more page table protection to ARM. As has been alluded to
in the past[1], the ARM kernel unconditionally maps everything as RWX which
presents a security problem. This is a fairly straight port of what we've been
using for a while for mitigation and is mostly provided as an example. Some
notes:

1) The original design mapped regions with permissions (RX/RO/RW) as appropriate
and then mapped the rest of the memory RW. I couldn't find a way to do this
cleanly in all cases so I went for the alternate approach: map all memory as RW,
clear the mapping and the set up the new mapping. This has some increased trade
offs described in #2

2) Rather than lose the TLB bonus for section mappings, the regions to be mapped
RO/RX are aligned to at least section size. For the new method of
clearing/remapping, this bumps up the alignment requirement to PMD_SIZE. This
does result in losing a bit of memory which is unfortunate.

3) This has not been tested on LPAE based systems

4) kprobes is broken with this since the text section is now RO. We had some
patches to allow temporarily marking the text writeable, but those did not take
into account section restrictions on SMP systems. I can post the patches if
there is interest.

Credit for the original work goes to Larry Bassel. I reworked parts of the
patches and added some new code.

Thanks,
Laura

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/187822.html

[RFC PATCH 1/5] arm: mm: add CONFIG_STRICT_MEMORY_RWX

[Laura Abbott]

From: Larry Bassel <lbassel@codeaurora.org>

If this is set, kernel text will be made RX, kernel data and stack
RW, rodata R so that writing to kernel text, executing kernel data
or stack, or writing to read-only data or kernel text will not
succeed.

Signed-off-by: Larry Bassel <lbassel@codeaurora.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
 arch/arm/mm/Kconfig |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index cd2c88e..c223d5c 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -952,3 +952,15 @@ config ARCH_HAS_BARRIERS
 	help
 	  This option allows the use of custom mandatory barriers
 	  included via the mach/barriers.h file.
+
+config STRICT_MEMORY_RWX
+	bool "restrict kernel memory permissions as much as possible"
+	default n
+	help
+	  If this is set, kernel text will be made RX, kernel data and stack
+	  RW, rodata R (otherwise all of the kernel 1-to-1 mapping is
+	  made RWX).
+	  The tradeoff is that several sections are padded to
+	  1M boundaries (because their permissions are different and
+	  splitting the 1M pages into 4K ones causes TLB performance
+	  problems), wasting memory.
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[RFC PATCH 1/5] arm: mm: add CONFIG_STRICT_MEMORY_RWX

[Larry Bassel]

On 08 Oct 13 18:31, Laura Abbott wrote:
> From: Larry Bassel <lbassel@codeaurora.org>
> 
> If this is set, kernel text will be made RX, kernel data and stack
> RW, rodata R so that writing to kernel text, executing kernel data
> or stack, or writing to read-only data or kernel text will not
> succeed.
> 
> Signed-off-by: Larry Bassel <lbassel@codeaurora.org>
> Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
> ---
>  arch/arm/mm/Kconfig |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
> index cd2c88e..c223d5c 100644
> --- a/arch/arm/mm/Kconfig
> +++ b/arch/arm/mm/Kconfig
> @@ -952,3 +952,15 @@ config ARCH_HAS_BARRIERS
>  	help
>  	  This option allows the use of custom mandatory barriers
>  	  included via the mach/barriers.h file.
> +
> +config STRICT_MEMORY_RWX
> +	bool "restrict kernel memory permissions as much as possible"
> +	default n
> +	help
> +	  If this is set, kernel text will be made RX, kernel data and stack
> +	  RW, rodata R (otherwise all of the kernel 1-to-1 mapping is
> +	  made RWX).
> +	  The tradeoff is that several sections are padded to
> +	  1M boundaries (because their permissions are different and

As this presumably (if it is accepted) will also need to run on
LPAE systems, we should say section not 1M here and below.

> +	  splitting the 1M pages into 4K ones causes TLB performance
> +	  problems), wasting memory.
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 

Larry

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[RFC PATCH 1/5] arm: mm: add CONFIG_STRICT_MEMORY_RWX

[Kees Cook]

On Tue, Oct 8, 2013 at 6:31 PM, Laura Abbott <lauraa@codeaurora.org> wrote:
> From: Larry Bassel <lbassel@codeaurora.org>
>
> If this is set, kernel text will be made RX, kernel data and stack
> RW, rodata R so that writing to kernel text, executing kernel data
> or stack, or writing to read-only data or kernel text will not
> succeed.
>
> Signed-off-by: Larry Bassel <lbassel@codeaurora.org>
> Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
> ---
>  arch/arm/mm/Kconfig |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
> index cd2c88e..c223d5c 100644
> --- a/arch/arm/mm/Kconfig
> +++ b/arch/arm/mm/Kconfig
> @@ -952,3 +952,15 @@ config ARCH_HAS_BARRIERS
>         help
>           This option allows the use of custom mandatory barriers
>           included via the mach/barriers.h file.
> +
> +config STRICT_MEMORY_RWX
> +       bool "restrict kernel memory permissions as much as possible"
> +       default n
> +       help
> +         If this is set, kernel text will be made RX, kernel data and stack
> +         RW, rodata R (otherwise all of the kernel 1-to-1 mapping is
> +         made RWX).
> +         The tradeoff is that several sections are padded to
> +         1M boundaries (because their permissions are different and
> +         splitting the 1M pages into 4K ones causes TLB performance
> +         problems), wasting memory.
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>

As a bike-shedding aside, would it make sense to attempt to follow the
x86 CONFIG naming convention for this? I don't like that x86's has
"DEBUG" in the name, but it might be nice for people looking for a
single name for these things. Unfortunately, x86 already has two:

CONFIG_DEBUG_RODATA
CONFIG_DEBUG_SET_MODULE_RONX

It'd be nice to have a common set, I think.

-Kees

-- 
Kees Cook
Chrome OS Security

[RFC PATCH 1/5] arm: mm: add CONFIG_STRICT_MEMORY_RWX

[Laura Abbott]

On 10/9/2013 1:04 PM, Kees Cook wrote:
> On Tue, Oct 8, 2013 at 6:31 PM, Laura Abbott <lauraa@codeaurora.org> wrote:
>> From: Larry Bassel <lbassel@codeaurora.org>
>>
>> If this is set, kernel text will be made RX, kernel data and stack
>> RW, rodata R so that writing to kernel text, executing kernel data
>> or stack, or writing to read-only data or kernel text will not
>> succeed.
>>
>> Signed-off-by: Larry Bassel <lbassel@codeaurora.org>
>> Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
>> ---
>>   arch/arm/mm/Kconfig |   12 ++++++++++++
>>   1 files changed, 12 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
>> index cd2c88e..c223d5c 100644
>> --- a/arch/arm/mm/Kconfig
>> +++ b/arch/arm/mm/Kconfig
>> @@ -952,3 +952,15 @@ config ARCH_HAS_BARRIERS
>>          help
>>            This option allows the use of custom mandatory barriers
>>            included via the mach/barriers.h file.
>> +
>> +config STRICT_MEMORY_RWX
>> +       bool "restrict kernel memory permissions as much as possible"
>> +       default n
>> +       help
>> +         If this is set, kernel text will be made RX, kernel data and stack
>> +         RW, rodata R (otherwise all of the kernel 1-to-1 mapping is
>> +         made RWX).
>> +         The tradeoff is that several sections are padded to
>> +         1M boundaries (because their permissions are different and
>> +         splitting the 1M pages into 4K ones causes TLB performance
>> +         problems), wasting memory.
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> hosted by The Linux Foundation
>>
>
> As a bike-shedding aside, would it make sense to attempt to follow the
> x86 CONFIG naming convention for this? I don't like that x86's has
> "DEBUG" in the name, but it might be nice for people looking for a
> single name for these things. Unfortunately, x86 already has two:
>
> CONFIG_DEBUG_RODATA
> CONFIG_DEBUG_SET_MODULE_RONX
>
> It'd be nice to have a common set, I think.
>

Agreed. My focus was on demonstrating the possibility of these features 
here. We may have to split things up a bit and or add another Kconfig. 
This is doing more than just RODATA though and the module work is 
separate (I sent patches out for that before and they never went 
anywhere because it was suggested to make the APIs less x86ish).
I'm sure there will be plenty of time for shedding bikes for lots of 
things in this series.

Laura

> -Kees
>


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[RFC PATCH 2/5] arm: mm: add new memory mapping types

[Laura Abbott]

From: Larry Bassel <lbassel@codeaurora.org>

Add mapping types similar to MT_MEMORY, except
that they are RW, RX or RO.

Signed-off-by: Larry Bassel <lbassel@codeaurora.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
 arch/arm/include/asm/mach/map.h |    3 +++
 arch/arm/mm/mmu.c               |   20 ++++++++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/mach/map.h b/arch/arm/include/asm/mach/map.h
index 2fe141f..322f1d8 100644
--- a/arch/arm/include/asm/mach/map.h
+++ b/arch/arm/include/asm/mach/map.h
@@ -34,6 +34,9 @@ struct map_desc {
 #define MT_MEMORY_ITCM		13
 #define MT_MEMORY_SO		14
 #define MT_MEMORY_DMA_READY	15
+#define MT_MEMORY_R		16
+#define MT_MEMORY_RW		17
+#define MT_MEMORY_RX		18
 
 #ifdef CONFIG_MMU
 extern void iotable_init(struct map_desc *, int);
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 53cdbd3..d846334 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -291,6 +291,21 @@ static struct mem_type mem_types[] = {
 		.prot_sect = PMD_TYPE_SECT | PMD_SECT_AP_WRITE,
 		.domain    = DOMAIN_KERNEL,
 	},
+	[MT_MEMORY_R] = {
+		.prot_l1   = PMD_TYPE_TABLE,
+		.prot_sect = PMD_TYPE_SECT | PMD_SECT_XN,
+		.domain    = DOMAIN_KERNEL,
+	},
+	[MT_MEMORY_RW] = {
+		.prot_l1   = PMD_TYPE_TABLE,
+		.prot_sect = PMD_TYPE_SECT | PMD_SECT_AP_WRITE | PMD_SECT_XN,
+		.domain    = DOMAIN_KERNEL,
+	},
+	[MT_MEMORY_RX] = {
+		.prot_l1   = PMD_TYPE_TABLE,
+		.prot_sect = PMD_TYPE_SECT,
+		.domain    = DOMAIN_KERNEL,
+	},
 	[MT_ROM] = {
 		.prot_sect = PMD_TYPE_SECT,
 		.domain    = DOMAIN_KERNEL,
@@ -468,6 +483,8 @@ static void __init build_mem_type_table(void)
 		 * from SVC mode and no access from userspace.
 		 */
 		mem_types[MT_ROM].prot_sect |= PMD_SECT_APX|PMD_SECT_AP_WRITE;
+		mem_types[MT_MEMORY_RX].prot_sect |= PMD_SECT_APX|PMD_SECT_AP_WRITE;
+		mem_types[MT_MEMORY_R].prot_sect |= PMD_SECT_APX|PMD_SECT_AP_WRITE;
 		mem_types[MT_MINICLEAN].prot_sect |= PMD_SECT_APX|PMD_SECT_AP_WRITE;
 		mem_types[MT_CACHECLEAN].prot_sect |= PMD_SECT_APX|PMD_SECT_AP_WRITE;
 #endif
@@ -545,6 +562,9 @@ static void __init build_mem_type_table(void)
 	mem_types[MT_MEMORY].prot_pte |= kern_pgprot;
 	mem_types[MT_MEMORY_DMA_READY].prot_pte |= kern_pgprot;
 	mem_types[MT_MEMORY_NONCACHED].prot_sect |= ecc_mask;
+	mem_types[MT_MEMORY_R].prot_sect |= ecc_mask | cp->pmd;
+	mem_types[MT_MEMORY_RW].prot_sect |= ecc_mask | cp->pmd;
+	mem_types[MT_MEMORY_RX].prot_sect |= ecc_mask | cp->pmd;
 	mem_types[MT_ROM].prot_sect |= cp->pmd;
 
 	switch (cp->pmd) {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[RFC PATCH 3/5] arm: align init, text, rodata to PMD_SIZE if CONFIG_STRICT_MEMORY_RWX is set

[Laura Abbott]

From: Larry Bassel <lbassel@codeaurora.org>

Init code, text, rodata and data need different permissions
and so they need to be on different pages. The kernel 1-to-1
mapping is constructed using section size pages to improve
TLB performance and this should not be changed (to 4K pages).
Therefore ensure that each of these regions starts on a PMD_SIZE
boundary. SECTION_SIZE is not enough as everything is mapped first
and then cleared later and remapped.

Signed-off-by: Larry Bassel <lbassel@codeaurora.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
 arch/arm/kernel/vmlinux.lds.S |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 7bcee5c..c06e915 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -8,6 +8,9 @@
 #include <asm/thread_info.h>
 #include <asm/memory.h>
 #include <asm/page.h>
+#ifdef CONFIG_STRICT_MEMORY_RWX
+#include <asm/pgtable.h>
+#endif
 	
 #define PROC_INFO							\
 	. = ALIGN(4);							\
@@ -90,6 +93,9 @@ SECTIONS
 		_text = .;
 		HEAD_TEXT
 	}
+#ifdef CONFIG_STRICT_MEMORY_RWX
+	. = ALIGN(1<<PMD_SHIFT);
+#endif
 	.text : {			/* Real text segment		*/
 		_stext = .;		/* Text and read-only data	*/
 			__exception_text_start = .;
@@ -112,6 +118,9 @@ SECTIONS
 			ARM_CPU_KEEP(PROC_INFO)
 	}
 
+#ifdef CONFIG_STRICT_MEMORY_RWX
+	. = ALIGN(1<<PMD_SHIFT);
+#endif
 	RO_DATA(PAGE_SIZE)
 
 	. = ALIGN(4);
@@ -145,7 +154,11 @@ SECTIONS
 	_etext = .;			/* End of text and rodata section */
 
 #ifndef CONFIG_XIP_KERNEL
+#ifdef CONFIG_STRICT_MEMORY_RWX
+	. = ALIGN(1<<PMD_SHIFT);
+#else
 	. = ALIGN(PAGE_SIZE);
+#endif
 	__init_begin = .;
 #endif
 	/*
@@ -173,6 +186,9 @@ SECTIONS
 	.init.proc.info : {
 		ARM_CPU_DISCARD(PROC_INFO)
 	}
+#ifdef CONFIG_STRICT_MEMORY_RWX
+	. = ALIGN(1<<PMD_SHIFT);
+#endif
 	.init.arch.info : {
 		__arch_info_begin = .;
 		*(.arch.info.init)
@@ -219,8 +235,12 @@ SECTIONS
 	__data_loc = ALIGN(4);		/* location in binary */
 	. = PAGE_OFFSET + TEXT_OFFSET;
 #else
-	__init_end = .;
+#ifdef CONFIG_STRICT_MEMORY_RWX
+	. = ALIGN(1<<PMD_SHIFT);
+#else
 	. = ALIGN(THREAD_SIZE);
+#endif
+	__init_end = .;
 	__data_loc = .;
 #endif
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[RFC PATCH 4/5] arm: mm: restrict kernel memory permissions if CONFIG_STRICT_MEMORY_RWX set

[Laura Abbott]

If CONFIG_STRICT_MEMORY_RWX is set, make kernel text RX,
kernel data/stack RW and rodata RO so that writing
on kernel text, executing kernel data or stack, or
writing on or executing read-only data is prohibited.

Signed-off-by: Larry Bassel <lbassel@codeaurora.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
 arch/arm/mm/mmu.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 56 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index d846334..91db2a0 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1311,6 +1311,60 @@ static void __init kmap_init(void)
 #endif
 }
 
+struct custom_map {
+	unsigned long start;
+	unsigned long end;
+	unsigned int type;
+};
+
+struct custom_map __initdata custom_maps[] = {
+	{
+		.start	= _stext,
+		.end	= __start_rodata,
+		.type	= MT_MEMORY_RX,
+	},
+	{
+		.start	= __start_rodata,
+		.end	= __init_begin,
+		.type	= MT_MEMORY_R
+	},
+	{
+		.start	= __init_begin,
+		.end	= __arch_info_begin,
+		.type	= MT_MEMORY_RX,
+	}
+};
+
+static void __init map_custom_regions(void)
+{
+#ifdef CONFIG_STRICT_MEMORY_RWX
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(custom_maps); i++) {
+		struct map_desc map;
+		unsigned long addr;
+
+		if (!IS_ALIGNED(custom_maps[i].start, PMD_SIZE) ||
+		    !IS_ALIGNED(custom_maps[i].end, PMD_SIZE)) {
+			pr_err("BUG: section %x-%x not aligned to %x\n",
+				custom_maps[i].start, custom_maps[i].end,
+				PMD_SIZE);
+			continue;
+		}
+
+		for (addr = custom_maps[i].start;
+			addr < custom_maps[i].end; addr += PMD_SIZE)
+				pmd_clear(pmd_off_k(addr));
+
+		map.virtual = custom_maps[i].start;
+		map.pfn = __phys_to_pfn(__virt_to_phys(custom_maps[i].start));
+		map.length = custom_maps[i].end - custom_maps[i].start;
+		map.type = custom_maps[i].type;
+		create_mapping(&map);
+	}
+#endif
+}
+
 static void __init map_lowmem(void)
 {
 	struct memblock_region *reg;
@@ -1329,10 +1383,11 @@ static void __init map_lowmem(void)
 		map.pfn = __phys_to_pfn(start);
 		map.virtual = __phys_to_virt(start);
 		map.length = end - start;
-		map.type = MT_MEMORY;
+		map.type = MT_MEMORY_RW;
 
 		create_mapping(&map);
 	}
+	map_custom_regions();
 }
 
 /*
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[RFC PATCH 5/5] arm: Don't free init text if CONFIG_STRICT_MEMORY_RWX is enabled

[Laura Abbott]

CONFIG_STRICT_MEMORY_RWX makes the text section be RX only. This
is incompatible with freeing the text back to general memory.
Skip the text free when freeing initmem. This does result in
less memory freed back into the system but we cannot easily
change the protections on the text section back to RWNX.

Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
Signed-off-by: Larry Bassel <lbassel@codeaurora.org>
---
 arch/arm/mm/init.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 15225d8..a23c99c 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -681,6 +681,9 @@ void __init mem_init(void)
 
 void free_initmem(void)
 {
+#ifdef CONFIG_STRICT_MEMORY_RWX
+	unsigned long reclaimed_initmem;
+#endif
 #ifdef CONFIG_HAVE_TCM
 	extern char __tcm_start, __tcm_end;
 
@@ -688,9 +691,18 @@ void free_initmem(void)
 	free_reserved_area(&__tcm_start, &__tcm_end, -1, "TCM link");
 #endif
 
+#ifdef CONFIG_STRICT_MEMORY_RWX
+	poison_init_mem((char *)__arch_info_begin,
+		__init_end - (char *)__arch_info_begin);
+	reclaimed_initmem = free_reserved_area(__arch_info_begin,
+					__init_end, -1,
+				    "init");
+	totalram_pages += reclaimed_initmem;
+#else
 	poison_init_mem(__init_begin, __init_end - __init_begin);
 	if (!machine_is_integrator() && !machine_is_cintegrator())
 		free_initmem_default(-1);
+#endif
 }
 
 #ifdef CONFIG_BLK_DEV_INITRD
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[RFC] Stricter kernel memory permissions

[Kees Cook]

On Tue, Oct 8, 2013 at 6:31 PM, Laura Abbott <lauraa@codeaurora.org> wrote:
> This is an RFC to add more page table protection to ARM. As has been alluded to
> in the past[1], the ARM kernel unconditionally maps everything as RWX which
> presents a security problem. This is a fairly straight port of what we've been
> using for a while for mitigation and is mostly provided as an example. Some
> notes:

On x86, there is CONFIG_X86_PTDUMP for providing a view of the kernel
page tables in /sys/kernel/debug/kernel_page_tables. Is there anything
like this for ARM? Or maybe the better question is "how are you
examining the results of your page permission changes?" :)

Thanks,

-Kees

-- 
Kees Cook
Chrome OS Security