On 11/30/2015 03:38 PM, Kees Cook wrote:
Given the choice between making things NX or making things RO, we want
RO first. As such, redefine CONFIG_DEBUG_RODATA to actually do the bulk
Can you give a citation for why? The thread that inspired it might be
a good link.
of the ROing (and NXing). In the place of the old CONFIG_DEBUG_RODATA,
introduce CONFIG_DEBUG_RODATA_ALIGN (after arm64's config that does the
same thing) to add the additional section alignment for making rodata
explicitly NX. Also adds human readable names to the sections so I
could more easily debug my typos, and makes CONFIG_DEBUG_RODATA default
"y" for CPU_V7.
Results in /sys/kernel/debug/kernel_page_tables for each config state:
# CONFIG_DEBUG_RODATA is not set
# CONFIG_DEBUG_ALIGN_RODATA is not set
---[ Kernel Mapping ]---
0x80000000-0x80900000 9M RW x SHD
0x80900000-0xa0000000 503M RW NX SHD
CONFIG_DEBUG_RODATA=y
CONFIG_DEBUG_ALIGN_RODATA=y
---[ Kernel Mapping ]---
0x80000000-0x80100000 1M RW NX SHD
0x80100000-0x80700000 6M ro x SHD
0x80700000-0x80a00000 3M ro NX SHD
0x80a00000-0xa0000000 502M RW NX SHD
CONFIG_DEBUG_RODATA=y
# CONFIG_DEBUG_ALIGN_RODATA is not set
---[ Kernel Mapping ]---
0x80000000-0x80100000 1M RW NX SHD
0x80100000-0x80a00000 9M ro x SHD
0x80a00000-0xa0000000 502M RW NX SHD
Signed-off-by: Kees Cook <[email protected]>
---
arch/arm/kernel/vmlinux.lds.S | 10 +++++-----
arch/arm/mm/Kconfig | 34 ++++++++++++++++++----------------
arch/arm/mm/init.c | 18 ++++++++++--------
3 files changed, 33 insertions(+), 29 deletions(-)
diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 8b60fde5ce48..a6e395c53a48 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -8,7 +8,7 @@
#include <asm/thread_info.h>
#include <asm/memory.h>
#include <asm/page.h>
-#ifdef CONFIG_ARM_KERNMEM_PERMS
+#ifdef CONFIG_DEBUG_RODATA
#include <asm/pgtable.h>
#endif
@@ -94,7 +94,7 @@ SECTIONS
HEAD_TEXT
}
-#ifdef CONFIG_ARM_KERNMEM_PERMS
+#ifdef CONFIG_DEBUG_RODATA
. = ALIGN(1<<SECTION_SHIFT);
#endif
@@ -117,7 +117,7 @@ SECTIONS
ARM_CPU_KEEP(PROC_INFO)
}
-#ifdef CONFIG_DEBUG_RODATA
+#ifdef CONFIG_DEBUG_ALIGN_RODATA
. = ALIGN(1<<SECTION_SHIFT);
#endif
RO_DATA(PAGE_SIZE)
@@ -153,7 +153,7 @@ SECTIONS
_etext = .; /* End of text and rodata section */
#ifndef CONFIG_XIP_KERNEL
-# ifdef CONFIG_ARM_KERNMEM_PERMS
+# ifdef CONFIG_DEBUG_RODATA
. = ALIGN(1<<SECTION_SHIFT);
# else
. = ALIGN(PAGE_SIZE);
@@ -231,7 +231,7 @@ SECTIONS
__data_loc = ALIGN(4); /* location in binary */
. = PAGE_OFFSET + TEXT_OFFSET;
#else
-#ifdef CONFIG_ARM_KERNMEM_PERMS
+#ifdef CONFIG_DEBUG_RODATA
. = ALIGN(1<<SECTION_SHIFT);
#else
. = ALIGN(THREAD_SIZE);
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 41218867a9a6..b617084e9520 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -1039,24 +1039,26 @@ config ARCH_SUPPORTS_BIG_ENDIAN
This option specifies the architecture can support big endian
operation.
-config ARM_KERNMEM_PERMS
- bool "Restrict kernel memory permissions"
+config DEBUG_RODATA
+ bool "Make kernel text and rodata read-only"
depends on MMU
+ default y if CPU_V7
help
- If this is set, kernel memory other than kernel text (and rodata)
- will be made non-executable. The tradeoff is that each region is
- padded to section-size (1MiB) boundaries (because their permissions
- are different and splitting the 1M pages into 4K ones causes TLB
- performance problems), wasting memory.
+ If this is set, kernel memory (text, rodata, etc) will be made
+ read-only, and non-text kernel memory will be made non-executable.
+ The tradeoff is that each region is padded to section-size (1MiB)
+ boundaries (because their permissions are different and splitting
+ the 1M pages into 4K ones causes TLB performance problems), which
+ can waste memory.
-config DEBUG_RODATA
- bool "Make kernel text and rodata read-only"
- depends on ARM_KERNMEM_PERMS
+config DEBUG_ALIGN_RODATA
+ bool "Make rodata strictly non-executable"
+ depends on DEBUG_RODATA
default y
help
- If this is set, kernel text and rodata will be made read-only. This
- is to help catch accidental or malicious attempts to change the
- kernel's executable code. Additionally splits rodata from kernel
- text so it can be made explicitly non-executable. This creates
- another section-size padded region, so it can waste more memory
- space while gaining the read-only protections.
+ If this is set, rodata will be made explicitly non-executable. This
+ provides protection on the rare chance that attackers might find and
+ use ROP gadgets that exist in the rodata section. This adds an
+ additional section-aligned split of rodata from kernel text so it
+ can be made explicitly non-executable. This padding may waste memory
+ space to gain this additional protection.
I get that you want to make this match arm64 but it's really not intuitive that
something with ALIGN_RODATA in the name is actually for setting NX. The purpose
of ALIGN_RODATA was also slightly different on arm64 since the RO/NX will still
be there, the difference is if the sections are present versus broken down into
pages.
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 8a63b4cdc0f2..e99f65fbcf2b 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -568,8 +568,9 @@ void __init mem_init(void)
}
}
-#ifdef CONFIG_ARM_KERNMEM_PERMS
+#ifdef CONFIG_DEBUG_RODATA
struct section_perm {
+ const char *name;
unsigned long start;
unsigned long end;
pmdval_t mask;
@@ -580,6 +581,7 @@ struct section_perm {
static struct section_perm nx_perms[] = {
/* Make pages tables, etc before _stext RW (set NX). */
{
+ .name = "pre-text NX",
.start = PAGE_OFFSET,
.end = (unsigned long)_stext,
.mask = ~PMD_SECT_XN,
@@ -587,14 +589,16 @@ static struct section_perm nx_perms[] = {
},
/* Make init RW (set NX). */
{
+ .name = "init NX",
.start = (unsigned long)__init_begin,
.end = (unsigned long)_sdata,
.mask = ~PMD_SECT_XN,
.prot = PMD_SECT_XN,
},
-#ifdef CONFIG_DEBUG_RODATA
+#ifdef CONFIG_DEBUG_ALIGN_RODATA
/* Make rodata NX (set RO in ro_perms below). */
{
+ .name = "rodata NX",
.start = (unsigned long)__start_rodata,
.end = (unsigned long)__init_begin,
.mask = ~PMD_SECT_XN,
@@ -603,10 +607,10 @@ static struct section_perm nx_perms[] = {
#endif
};
-#ifdef CONFIG_DEBUG_RODATA
static struct section_perm ro_perms[] = {
/* Make kernel code and rodata RX (set RO). */
{
+ .name = "text/rodata RO",
.start = (unsigned long)_stext,
.end = (unsigned long)__init_begin,
#ifdef CONFIG_ARM_LPAE
@@ -619,7 +623,6 @@ static struct section_perm ro_perms[] = {
#endif
},
};
-#endif
/*
* Updates section permissions only for the current mm (sections are
@@ -666,7 +669,8 @@ static inline bool arch_has_strict_perms(void)
for (i = 0; i < ARRAY_SIZE(perms); i++) { \
if (!IS_ALIGNED(perms[i].start, SECTION_SIZE) || \
!IS_ALIGNED(perms[i].end, SECTION_SIZE)) { \
- pr_err("BUG: section %lx-%lx not aligned to %lx\n", \
+ pr_err("BUG: %s section %lx-%lx not aligned to %lx\n", \
+ perms[i].name, \
perms[i].start, perms[i].end, \
SECTION_SIZE); \
continue; \
@@ -685,7 +689,6 @@ static inline void fix_kernmem_perms(void)
set_section_perms(nx_perms, prot);
}
-#ifdef CONFIG_DEBUG_RODATA
void mark_rodata_ro(void)
{
set_section_perms(ro_perms, prot);
@@ -700,11 +703,10 @@ void set_kernel_text_ro(void)
{
set_section_perms(ro_perms, prot);
}
-#endif /* CONFIG_DEBUG_RODATA */
#else
static inline void fix_kernmem_perms(void) { }
-#endif /* CONFIG_ARM_KERNMEM_PERMS */
+#endif /* CONFIG_DEBUG_RODATA */
void free_tcmmem(void)
{
Thanks,
Laura
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
