linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH 0/2] Better hardening names
@ 2017-01-19  1:29 Laura Abbott
  2017-01-19  1:29 ` [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA Laura Abbott
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Laura Abbott @ 2017-01-19  1:29 UTC (permalink / raw)
  To: Kees Cook
  Cc: Laura Abbott, Jason Wessel, Jonathan Corbet, Russell King,
	Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Pavel Machek, Mark Rutland, Jessica Yu, linux-doc, linux-kernel,
	linux-arm-kernel, linux-parisc, linux-s390, linux-pm,
	kernel-hardening

Hi,

It's come up previously that CONFIG_DEBUG_SET_MODULE_RONX and
CONFIG_DEBUG_RODATA are not accurate names, mostly they should not have the
word 'debug' in them. This series attempts to change the names to something
a bit more descriptive and indicative of what they are actually used for these
days.

I marked this RFC for
- Bike shedding purposes.
- A discussion of what defaults should be. The way I did the refactoring, both
  options are default y. I'd appreciate comments if there is a better approach.
- Approach to split this up into more sub patches to make review/merging easier?
  Or maybe it's fine.

Quickly tested on arm/arm64/x86.

Thanks,
Laura

Laura Abbott (2):
  security: Change name of CONFIG_DEBUG_RODATA
  security: Change name of CONFIG_DEBUG_SET_MODULE_RONX

 Documentation/DocBook/kgdb.tmpl            |  8 ++++----
 Documentation/security/self-protection.txt |  4 ++--
 arch/arm/Kconfig                           |  2 ++
 arch/arm/Kconfig.debug                     | 11 ----------
 arch/arm/configs/aspeed_g4_defconfig       |  4 ++--
 arch/arm/configs/aspeed_g5_defconfig       |  4 ++--
 arch/arm/include/asm/cacheflush.h          |  2 +-
 arch/arm/kernel/patch.c                    |  4 ++--
 arch/arm/kernel/vmlinux.lds.S              |  8 ++++----
 arch/arm/mm/Kconfig                        | 14 +------------
 arch/arm/mm/init.c                         |  4 ++--
 arch/arm64/Kconfig                         |  5 ++---
 arch/arm64/Kconfig.debug                   | 13 +-----------
 arch/arm64/kernel/insn.c                   |  2 +-
 arch/parisc/Kconfig                        |  1 +
 arch/parisc/Kconfig.debug                  | 11 ----------
 arch/parisc/configs/712_defconfig          |  2 +-
 arch/parisc/configs/c3000_defconfig        |  2 +-
 arch/parisc/mm/init.c                      |  2 +-
 arch/s390/Kconfig                          |  5 ++---
 arch/s390/Kconfig.debug                    |  3 ---
 arch/x86/Kconfig                           |  5 ++---
 arch/x86/Kconfig.debug                     | 11 ----------
 include/linux/filter.h                     |  4 ++--
 include/linux/init.h                       |  4 ++--
 include/linux/module.h                     |  2 +-
 init/main.c                                |  4 ++--
 kernel/configs/android-recommended.config  |  2 +-
 kernel/module.c                            |  6 +++---
 kernel/power/hibernate.c                   |  2 +-
 kernel/power/power.h                       |  4 ++--
 kernel/power/snapshot.c                    |  4 ++--
 security/Kconfig                           | 32 ++++++++++++++++++++++++++++++
 33 files changed, 82 insertions(+), 109 deletions(-)

-- 
2.7.4

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

* [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA
  2017-01-19  1:29 [RFC][PATCH 0/2] Better hardening names Laura Abbott
@ 2017-01-19  1:29 ` Laura Abbott
  2017-01-19  7:53   ` Pavel Machek
                     ` (2 more replies)
  2017-01-19  1:29 ` [PATCH 2/2] security: Change name of CONFIG_DEBUG_SET_MODULE_RONX Laura Abbott
  2017-01-19 22:08 ` [RFC][PATCH 0/2] Better hardening names Kees Cook
  2 siblings, 3 replies; 19+ messages in thread
From: Laura Abbott @ 2017-01-19  1:29 UTC (permalink / raw)
  To: Kees Cook
  Cc: Laura Abbott, Jason Wessel, Jonathan Corbet, Russell King,
	Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Pavel Machek, Mark Rutland, Jessica Yu, linux-doc, linux-kernel,
	linux-arm-kernel, linux-parisc, linux-s390, linux-pm,
	kernel-hardening


Despite the word 'debug' in CONFIG_DEBUG_RODATA, this kernel option
provides key security features that are to be expected on a modern
system. Change the name to CONFIG_HARDENED_PAGE_MAPPINGS which more
accurately describes what this option is intended to do.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 Documentation/DocBook/kgdb.tmpl            |  8 ++++----
 Documentation/security/self-protection.txt |  2 +-
 arch/arm/Kconfig                           |  1 +
 arch/arm/configs/aspeed_g4_defconfig       |  2 +-
 arch/arm/configs/aspeed_g5_defconfig       |  2 +-
 arch/arm/include/asm/cacheflush.h          |  2 +-
 arch/arm/kernel/patch.c                    |  2 +-
 arch/arm/kernel/vmlinux.lds.S              |  8 ++++----
 arch/arm/mm/Kconfig                        | 14 +-------------
 arch/arm/mm/init.c                         |  4 ++--
 arch/arm64/Kconfig                         |  4 +---
 arch/arm64/Kconfig.debug                   |  2 +-
 arch/parisc/Kconfig                        |  1 +
 arch/parisc/Kconfig.debug                  | 11 -----------
 arch/parisc/configs/712_defconfig          |  2 +-
 arch/parisc/configs/c3000_defconfig        |  2 +-
 arch/parisc/mm/init.c                      |  2 +-
 arch/s390/Kconfig                          |  4 +---
 arch/x86/Kconfig                           |  4 +---
 include/linux/init.h                       |  4 ++--
 init/main.c                                |  4 ++--
 kernel/configs/android-recommended.config  |  2 +-
 kernel/power/hibernate.c                   |  2 +-
 kernel/power/power.h                       |  4 ++--
 kernel/power/snapshot.c                    |  4 ++--
 security/Kconfig                           | 16 ++++++++++++++++
 26 files changed, 51 insertions(+), 62 deletions(-)

diff --git a/Documentation/DocBook/kgdb.tmpl b/Documentation/DocBook/kgdb.tmpl
index f3abca7..a79b638 100644
--- a/Documentation/DocBook/kgdb.tmpl
+++ b/Documentation/DocBook/kgdb.tmpl
@@ -115,12 +115,12 @@
     </para>
     <para>
     If the architecture that you are using supports the kernel option
-    CONFIG_DEBUG_RODATA, you should consider turning it off.  This
+    CONFIG_HARDENED_PAGE_MAPPINGS, you should consider turning it off.  This
     option will prevent the use of software breakpoints because it
     marks certain regions of the kernel's memory space as read-only.
     If kgdb supports it for the architecture you are using, you can
     use hardware breakpoints if you desire to run with the
-    CONFIG_DEBUG_RODATA option turned on, else you need to turn off
+    CONFIG_HARDENED_PAGE_MAPPINGS option turned on, else you need to turn off
     this option.
     </para>
     <para>
@@ -135,7 +135,7 @@
     <para>Here is an example set of .config symbols to enable or
     disable for kgdb:
     <itemizedlist>
-    <listitem><para># CONFIG_DEBUG_RODATA is not set</para></listitem>
+    <listitem><para># CONFIG_HARDENED_PAGE_MAPPINGS is not set</para></listitem>
     <listitem><para>CONFIG_FRAME_POINTER=y</para></listitem>
     <listitem><para>CONFIG_KGDB=y</para></listitem>
     <listitem><para>CONFIG_KGDB_SERIAL_CONSOLE=y</para></listitem>
@@ -166,7 +166,7 @@
     </para>
     <para>Here is an example set of .config symbols to enable/disable kdb:
     <itemizedlist>
-    <listitem><para># CONFIG_DEBUG_RODATA is not set</para></listitem>
+    <listitem><para># CONFIG_HARDENED_PAGE_MAPPINGS is not set</para></listitem>
     <listitem><para>CONFIG_FRAME_POINTER=y</para></listitem>
     <listitem><para>CONFIG_KGDB=y</para></listitem>
     <listitem><para>CONFIG_KGDB_SERIAL_CONSOLE=y</para></listitem>
diff --git a/Documentation/security/self-protection.txt b/Documentation/security/self-protection.txt
index 3010576..da8cb36 100644
--- a/Documentation/security/self-protection.txt
+++ b/Documentation/security/self-protection.txt
@@ -51,7 +51,7 @@ kernel, they are implemented in a way where the memory is temporarily
 made writable during the update, and then returned to the original
 permissions.)
 
-In support of this are (the poorly named) CONFIG_DEBUG_RODATA and
+In support of this are CONFIG_HARDENED_PAGE_MAPPINGS and
 CONFIG_DEBUG_SET_MODULE_RONX, which seek to make sure that code is not
 writable, data is not executable, and read-only data is neither writable
 nor executable.
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 186c4c2..09aff28 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -7,6 +7,7 @@ config ARM
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
 	select ARCH_HAVE_CUSTOM_GPIO_H
 	select ARCH_HAS_GCOV_PROFILE_ALL
+	select ARCH_HAS_HARDENED_MAPPINGS if MMU && !XIP_KERNEL
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select ARCH_SUPPORTS_ATOMIC_RMW
 	select ARCH_USE_BUILTIN_BSWAP
diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig
index ca39c04..8ccc216 100644
--- a/arch/arm/configs/aspeed_g4_defconfig
+++ b/arch/arm/configs/aspeed_g4_defconfig
@@ -25,7 +25,7 @@ CONFIG_MODULE_UNLOAD=y
 # CONFIG_ARCH_MULTI_V7 is not set
 CONFIG_ARCH_ASPEED=y
 CONFIG_MACH_ASPEED_G4=y
-CONFIG_DEBUG_RODATA=y
+CONFIG_HARDENED_PAGE_MAPPINGS=y
 CONFIG_AEABI=y
 CONFIG_UACCESS_WITH_MEMCPY=y
 CONFIG_SECCOMP=y
diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index 4f366b0..90c5ce4 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -26,7 +26,7 @@ CONFIG_ARCH_MULTI_V6=y
 # CONFIG_ARCH_MULTI_V7 is not set
 CONFIG_ARCH_ASPEED=y
 CONFIG_MACH_ASPEED_G5=y
-CONFIG_DEBUG_RODATA=y
+CONFIG_HARDENED_PAGE_MAPPINGS=y
 CONFIG_AEABI=y
 CONFIG_UACCESS_WITH_MEMCPY=y
 CONFIG_SECCOMP=y
diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
index bdd283b..c3a7a72 100644
--- a/arch/arm/include/asm/cacheflush.h
+++ b/arch/arm/include/asm/cacheflush.h
@@ -490,7 +490,7 @@ static inline int set_memory_x(unsigned long addr, int numpages) { return 0; }
 static inline int set_memory_nx(unsigned long addr, int numpages) { return 0; }
 #endif
 
-#ifdef CONFIG_DEBUG_RODATA
+#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
 void set_kernel_text_rw(void);
 void set_kernel_text_ro(void);
 #else
diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
index 69bda1a..9da1bf5 100644
--- a/arch/arm/kernel/patch.c
+++ b/arch/arm/kernel/patch.c
@@ -26,7 +26,7 @@ static void __kprobes *patch_map(void *addr, int fixmap, unsigned long *flags)
 
 	if (module && IS_ENABLED(CONFIG_DEBUG_SET_MODULE_RONX))
 		page = vmalloc_to_page(addr);
-	else if (!module && IS_ENABLED(CONFIG_DEBUG_RODATA))
+	else if (!module && IS_ENABLED(CONFIG_HARDENED_PAGE_MAPPINGS))
 		page = virt_to_page(addr);
 	else
 		return addr;
diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index f7f55df..5c6a2e8 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -97,7 +97,7 @@ SECTIONS
 		HEAD_TEXT
 	}
 
-#ifdef CONFIG_DEBUG_RODATA
+#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
 	. = ALIGN(1<<SECTION_SHIFT);
 #endif
 
@@ -158,7 +158,7 @@ SECTIONS
 
 	NOTES
 
-#ifdef CONFIG_DEBUG_RODATA
+#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
 	. = ALIGN(1<<SECTION_SHIFT);
 #else
 	. = ALIGN(PAGE_SIZE);
@@ -230,7 +230,7 @@ SECTIONS
 	PERCPU_SECTION(L1_CACHE_BYTES)
 #endif
 
-#ifdef CONFIG_DEBUG_RODATA
+#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
 	. = ALIGN(1<<SECTION_SHIFT);
 #else
 	. = ALIGN(THREAD_SIZE);
@@ -325,7 +325,7 @@ SECTIONS
 	STABS_DEBUG
 }
 
-#ifdef CONFIG_DEBUG_RODATA
+#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
 /*
  * Without CONFIG_DEBUG_ALIGN_RODATA, __start_rodata_section_aligned will
  * be the first section-aligned location after __start_rodata. Otherwise,
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index f68e8ec..e770dc9 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -1051,21 +1051,9 @@ config ARCH_SUPPORTS_BIG_ENDIAN
 	  This option specifies the architecture can support big endian
 	  operation.
 
-config DEBUG_RODATA
-	bool "Make kernel text and rodata read-only"
-	depends on MMU && !XIP_KERNEL
-	default y if CPU_V7
-	help
-	  If this is set, kernel text and rodata memory 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_ALIGN_RODATA
 	bool "Make rodata strictly non-executable"
-	depends on DEBUG_RODATA
+	depends on HARDENED_PAGE_MAPPINGS
 	default y
 	help
 	  If this is set, rodata will be made explicitly non-executable. This
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 370581a..303bee4 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -572,7 +572,7 @@ void __init mem_init(void)
 	}
 }
 
-#ifdef CONFIG_DEBUG_RODATA
+#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
 struct section_perm {
 	const char *name;
 	unsigned long start;
@@ -741,7 +741,7 @@ void set_kernel_text_ro(void)
 
 #else
 static inline void fix_kernmem_perms(void) { }
-#endif /* CONFIG_DEBUG_RODATA */
+#endif /* CONFIG_HARDENED_PAGE_MAPPINGS */
 
 void free_tcmmem(void)
 {
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1117421..06fed56 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -11,6 +11,7 @@ config ARM64
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_HAS_GIGANTIC_PAGE
+	select ARCH_HAS_HARDENED_MAPPINGS
 	select ARCH_HAS_KCOV
 	select ARCH_HAS_SG_CHAIN
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
@@ -123,9 +124,6 @@ config ARCH_PHYS_ADDR_T_64BIT
 config MMU
 	def_bool y
 
-config DEBUG_RODATA
-	def_bool y
-
 config ARM64_PAGE_SHIFT
 	int
 	default 16 if ARM64_64K_PAGES
diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
index d1ebd46..a26d27f 100644
--- a/arch/arm64/Kconfig.debug
+++ b/arch/arm64/Kconfig.debug
@@ -83,7 +83,7 @@ config DEBUG_SET_MODULE_RONX
 	  If in doubt, say Y.
 
 config DEBUG_ALIGN_RODATA
-	depends on DEBUG_RODATA
+	depends on ARCH_HAS_HARDENED_MAPPINGS
 	bool "Align linker sections up to SECTION_SIZE"
 	help
 	  If this option is enabled, sections that may potentially be marked as
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 3a71f38..7c73eaa 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -8,6 +8,7 @@ config PARISC
 	select HAVE_SYSCALL_TRACEPOINTS
 	select ARCH_WANT_FRAME_POINTERS
 	select ARCH_HAS_ELF_RANDOMIZE
+	select ARCH_HAS_HARDENED_MAPPINGS
 	select RTC_CLASS
 	select RTC_DRV_GENERIC
 	select INIT_ALL_POSSIBLE
diff --git a/arch/parisc/Kconfig.debug b/arch/parisc/Kconfig.debug
index 68b7cbd..0d856b9 100644
--- a/arch/parisc/Kconfig.debug
+++ b/arch/parisc/Kconfig.debug
@@ -5,15 +5,4 @@ source "lib/Kconfig.debug"
 config TRACE_IRQFLAGS_SUPPORT
 	def_bool y
 
-config DEBUG_RODATA
-       bool "Write protect kernel read-only data structures"
-       depends on DEBUG_KERNEL
-       default y
-       help
-         Mark the kernel read-only data as write-protected in the pagetables,
-         in order to catch accidental (and incorrect) writes to such const
-         data. This option may have a slight performance impact because a
-         portion of the kernel code won't be covered by a TLB anymore.
-         If in doubt, say "N".
-
 endmenu
diff --git a/arch/parisc/configs/712_defconfig b/arch/parisc/configs/712_defconfig
index db8f56b..f1a4732 100644
--- a/arch/parisc/configs/712_defconfig
+++ b/arch/parisc/configs/712_defconfig
@@ -182,7 +182,7 @@ CONFIG_DEBUG_FS=y
 CONFIG_DEBUG_KERNEL=y
 CONFIG_DEBUG_MUTEXES=y
 # CONFIG_RCU_CPU_STALL_DETECTOR is not set
-CONFIG_DEBUG_RODATA=y
+CONFIG_HARDENED_PAGE_MAPPINGS=y
 CONFIG_CRYPTO_NULL=m
 CONFIG_CRYPTO_TEST=m
 CONFIG_CRYPTO_HMAC=y
diff --git a/arch/parisc/configs/c3000_defconfig b/arch/parisc/configs/c3000_defconfig
index fb92b89..03e88e2 100644
--- a/arch/parisc/configs/c3000_defconfig
+++ b/arch/parisc/configs/c3000_defconfig
@@ -166,7 +166,7 @@ CONFIG_DEBUG_KERNEL=y
 CONFIG_DEBUG_MUTEXES=y
 # CONFIG_DEBUG_BUGVERBOSE is not set
 # CONFIG_RCU_CPU_STALL_DETECTOR is not set
-CONFIG_DEBUG_RODATA=y
+CONFIG_HARDENED_PAGE_MAPPINGS=y
 CONFIG_CRYPTO_NULL=m
 CONFIG_CRYPTO_TEST=m
 CONFIG_CRYPTO_MD5=m
diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
index e02ada3..f76f8ad 100644
--- a/arch/parisc/mm/init.c
+++ b/arch/parisc/mm/init.c
@@ -545,7 +545,7 @@ void free_initmem(void)
 }
 
 
-#ifdef CONFIG_DEBUG_RODATA
+#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
 void mark_rodata_ro(void)
 {
 	/* rodata memory was already mapped with KERNEL_RO access rights by
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index c6722112..8e70ae5 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -62,15 +62,13 @@ config PCI_QUIRKS
 config ARCH_SUPPORTS_UPROBES
 	def_bool y
 
-config DEBUG_RODATA
-	def_bool y
-
 config S390
 	def_bool y
 	select ARCH_HAS_DEVMEM_IS_ALLOWED
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_HAS_GIGANTIC_PAGE
+	select ARCH_HAS_HARDENED_MAPPINGS
 	select ARCH_HAS_KCOV
 	select ARCH_HAS_SG_CHAIN
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index e487493..9d80cd8 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -50,6 +50,7 @@ config X86
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_FAST_MULTIPLIER
 	select ARCH_HAS_GCOV_PROFILE_ALL
+	select ARCH_HAS_HARDENED_MAPPINGS
 	select ARCH_HAS_KCOV			if X86_64
 	select ARCH_HAS_MMIO_FLUSH
 	select ARCH_HAS_PMEM_API		if X86_64
@@ -309,9 +310,6 @@ config ARCH_SUPPORTS_UPROBES
 config FIX_EARLYCON_MEM
 	def_bool y
 
-config DEBUG_RODATA
-	def_bool y
-
 config PGTABLE_LEVELS
 	int
 	default 4 if X86_64
diff --git a/include/linux/init.h b/include/linux/init.h
index 885c3e6..9967bc9 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -126,10 +126,10 @@ void prepare_namespace(void);
 void __init load_default_modules(void);
 int __init init_rootfs(void);
 
-#if defined(CONFIG_DEBUG_RODATA) || defined(CONFIG_DEBUG_SET_MODULE_RONX)
+#if defined(CONFIG_HARDENED_PAGE_MAPPINGS) || defined(CONFIG_DEBUG_SET_MODULE_RONX)
 extern bool rodata_enabled;
 #endif
-#ifdef CONFIG_DEBUG_RODATA
+#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
 void mark_rodata_ro(void);
 #endif
 
diff --git a/init/main.c b/init/main.c
index b0c9d6f..4b3bcc4 100644
--- a/init/main.c
+++ b/init/main.c
@@ -925,7 +925,7 @@ static int try_to_run_init_process(const char *init_filename)
 
 static noinline void __init kernel_init_freeable(void);
 
-#if defined(CONFIG_DEBUG_RODATA) || defined(CONFIG_DEBUG_SET_MODULE_RONX)
+#if defined(CONFIG_HARDENED_PAGE_MAPPINGS) || defined(CONFIG_DEBUG_SET_MODULE_RONX)
 bool rodata_enabled __ro_after_init = true;
 static int __init set_debug_rodata(char *str)
 {
@@ -934,7 +934,7 @@ static int __init set_debug_rodata(char *str)
 __setup("rodata=", set_debug_rodata);
 #endif
 
-#ifdef CONFIG_DEBUG_RODATA
+#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
 static void mark_readonly(void)
 {
 	if (rodata_enabled)
diff --git a/kernel/configs/android-recommended.config b/kernel/configs/android-recommended.config
index 297756b..b796bc8 100644
--- a/kernel/configs/android-recommended.config
+++ b/kernel/configs/android-recommended.config
@@ -11,7 +11,7 @@ CONFIG_BLK_DEV_LOOP=y
 CONFIG_BLK_DEV_RAM=y
 CONFIG_BLK_DEV_RAM_SIZE=8192
 CONFIG_COMPACTION=y
-CONFIG_DEBUG_RODATA=y
+CONFIG_HARDENED_PAGE_MAPPINGS=y
 CONFIG_DM_CRYPT=y
 CONFIG_DM_UEVENT=y
 CONFIG_DM_VERITY=y
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index b26dbc4..f7a3ea3 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -1156,7 +1156,7 @@ static int __init hibernate_setup(char *str)
 	} else if (!strncmp(str, "no", 2)) {
 		noresume = 1;
 		nohibernate = 1;
-	} else if (IS_ENABLED(CONFIG_DEBUG_RODATA)
+	} else if (IS_ENABLED(CONFIG_HARDENED_PAGE_MAPPINGS)
 		   && !strncmp(str, "protect_image", 13)) {
 		enable_restore_image_protection();
 	}
diff --git a/kernel/power/power.h b/kernel/power/power.h
index 1dfa0da..dc2c7b8 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -61,12 +61,12 @@ extern int hibernation_snapshot(int platform_mode);
 extern int hibernation_restore(int platform_mode);
 extern int hibernation_platform_enter(void);
 
-#ifdef CONFIG_DEBUG_RODATA
+#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
 /* kernel/power/snapshot.c */
 extern void enable_restore_image_protection(void);
 #else
 static inline void enable_restore_image_protection(void) {}
-#endif /* CONFIG_DEBUG_RODATA */
+#endif /* CONFIG_HARDENED_PAGE_MAPPINGS */
 
 #else /* !CONFIG_HIBERNATION */
 
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 2d8e2b2..a7c793e 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -38,7 +38,7 @@
 
 #include "power.h"
 
-#ifdef CONFIG_DEBUG_RODATA
+#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
 static bool hibernate_restore_protection;
 static bool hibernate_restore_protection_active;
 
@@ -73,7 +73,7 @@ static inline void hibernate_restore_protection_begin(void) {}
 static inline void hibernate_restore_protection_end(void) {}
 static inline void hibernate_restore_protect_page(void *page_address) {}
 static inline void hibernate_restore_unprotect_page(void *page_address) {}
-#endif /* CONFIG_DEBUG_RODATA */
+#endif /* CONFIG_HARDENED_PAGE_MAPPINGS */
 
 static int swsusp_page_is_free(struct page *);
 static void swsusp_set_page_forbidden(struct page *);
diff --git a/security/Kconfig b/security/Kconfig
index 118f454..ad6ce82 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -158,6 +158,22 @@ config HARDENED_USERCOPY_PAGESPAN
 	  been removed. This config is intended to be used only while
 	  trying to find such users.
 
+config ARCH_HAS_HARDENED_MAPPINGS
+	def_bool n
+
+config HARDENED_PAGE_MAPPINGS
+	bool "Mark kernel mappings with stricter permissions (RO/W^X)"
+	default y
+	depends on ARCH_HAS_HARDENED_MAPPINGS
+	help
+          If this is set, kernel text and rodata memory will be made read-only,
+	  and non-text memory will be made non-executable. This provides
+	  protection against certain security attacks (e.g. executing the heap
+	  or modifying text).
+
+	  Unless your system has known restrictions or performance issues, it
+	  is recommended to say Y here.
+
 source security/selinux/Kconfig
 source security/smack/Kconfig
 source security/tomoyo/Kconfig
-- 
2.7.4

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

* [PATCH 2/2] security: Change name of CONFIG_DEBUG_SET_MODULE_RONX
  2017-01-19  1:29 [RFC][PATCH 0/2] Better hardening names Laura Abbott
  2017-01-19  1:29 ` [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA Laura Abbott
@ 2017-01-19  1:29 ` Laura Abbott
  2017-01-19 11:11   ` Mark Rutland
                     ` (2 more replies)
  2017-01-19 22:08 ` [RFC][PATCH 0/2] Better hardening names Kees Cook
  2 siblings, 3 replies; 19+ messages in thread
From: Laura Abbott @ 2017-01-19  1:29 UTC (permalink / raw)
  To: Kees Cook
  Cc: Laura Abbott, Jason Wessel, Jonathan Corbet, Russell King,
	Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Pavel Machek, Mark Rutland, Jessica Yu, linux-doc, linux-kernel,
	linux-arm-kernel, linux-parisc, linux-s390, linux-pm,
	kernel-hardening


Despite the word 'debug' in CONFIG_DEBUG_SET_MODULE_RONX, this kernel
option provides key security features that are to be expected on a
modern system. Change the name to CONFIG_HARDENED_MODULE_MAPPINGS which
more accurately describes what this option is intended to do.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 Documentation/security/self-protection.txt |  2 +-
 arch/arm/Kconfig                           |  1 +
 arch/arm/Kconfig.debug                     | 11 -----------
 arch/arm/configs/aspeed_g4_defconfig       |  2 +-
 arch/arm/configs/aspeed_g5_defconfig       |  2 +-
 arch/arm/kernel/patch.c                    |  2 +-
 arch/arm64/Kconfig                         |  1 +
 arch/arm64/Kconfig.debug                   | 11 -----------
 arch/arm64/kernel/insn.c                   |  2 +-
 arch/s390/Kconfig                          |  1 +
 arch/s390/Kconfig.debug                    |  3 ---
 arch/x86/Kconfig                           |  1 +
 arch/x86/Kconfig.debug                     | 11 -----------
 include/linux/filter.h                     |  4 ++--
 include/linux/init.h                       |  2 +-
 include/linux/module.h                     |  2 +-
 init/main.c                                |  2 +-
 kernel/module.c                            |  6 +++---
 security/Kconfig                           | 16 ++++++++++++++++
 19 files changed, 33 insertions(+), 49 deletions(-)

diff --git a/Documentation/security/self-protection.txt b/Documentation/security/self-protection.txt
index da8cb36..eb018a1 100644
--- a/Documentation/security/self-protection.txt
+++ b/Documentation/security/self-protection.txt
@@ -52,7 +52,7 @@ made writable during the update, and then returned to the original
 permissions.)
 
 In support of this are CONFIG_HARDENED_PAGE_MAPPINGS and
-CONFIG_DEBUG_SET_MODULE_RONX, which seek to make sure that code is not
+CONFIG_HARDENED_MODULE_MAPPINGS, which seek to make sure that code is not
 writable, data is not executable, and read-only data is neither writable
 nor executable.
 
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 09aff28..ef852e4 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -8,6 +8,7 @@ config ARM
 	select ARCH_HAVE_CUSTOM_GPIO_H
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_HAS_HARDENED_MAPPINGS if MMU && !XIP_KERNEL
+	select ARCH_HAS_HARDENED_MODULE_MAPPINGS if MMU
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select ARCH_SUPPORTS_ATOMIC_RMW
 	select ARCH_USE_BUILTIN_BSWAP
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index d83f7c3..426d271 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -1738,17 +1738,6 @@ config PID_IN_CONTEXTIDR
 	  additional instructions during context switch. Say Y here only if you
 	  are planning to use hardware trace tools with this kernel.
 
-config DEBUG_SET_MODULE_RONX
-	bool "Set loadable kernel module data as NX and text as RO"
-	depends on MODULES && MMU
-	---help---
-	  This option helps catch unintended modifications to loadable
-	  kernel module's text and read-only data. It also prevents execution
-	  of module data. Such protection may interfere with run-time code
-	  patching and dynamic kernel tracing - and they might also protect
-	  against certain classes of kernel exploits.
-	  If in doubt, say "N".
-
 source "drivers/hwtracing/coresight/Kconfig"
 
 endmenu
diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig
index 8ccc216..ffe2656 100644
--- a/arch/arm/configs/aspeed_g4_defconfig
+++ b/arch/arm/configs/aspeed_g4_defconfig
@@ -79,7 +79,7 @@ CONFIG_DEBUG_LL_UART_8250=y
 CONFIG_DEBUG_UART_PHYS=0x1e784000
 CONFIG_DEBUG_UART_VIRT=0xe8784000
 CONFIG_EARLY_PRINTK=y
-CONFIG_DEBUG_SET_MODULE_RONX=y
+CONFIG_HARDENED_MODULE_MAPPINGS=y
 # CONFIG_XZ_DEC_X86 is not set
 # CONFIG_XZ_DEC_POWERPC is not set
 # CONFIG_XZ_DEC_IA64 is not set
diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index 90c5ce4..2ea444e 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -81,7 +81,7 @@ CONFIG_DEBUG_LL_UART_8250=y
 CONFIG_DEBUG_UART_PHYS=0x1e784000
 CONFIG_DEBUG_UART_VIRT=0xe8784000
 CONFIG_EARLY_PRINTK=y
-CONFIG_DEBUG_SET_MODULE_RONX=y
+CONFIG_HARDENED_MODULE_MAPPINGS=y
 # CONFIG_XZ_DEC_X86 is not set
 # CONFIG_XZ_DEC_POWERPC is not set
 # CONFIG_XZ_DEC_IA64 is not set
diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
index 9da1bf5..eb73a76 100644
--- a/arch/arm/kernel/patch.c
+++ b/arch/arm/kernel/patch.c
@@ -24,7 +24,7 @@ static void __kprobes *patch_map(void *addr, int fixmap, unsigned long *flags)
 	bool module = !core_kernel_text(uintaddr);
 	struct page *page;
 
-	if (module && IS_ENABLED(CONFIG_DEBUG_SET_MODULE_RONX))
+	if (module && IS_ENABLED(CONFIG_HARDENED_MODULE_MAPPINGS))
 		page = vmalloc_to_page(addr);
 	else if (!module && IS_ENABLED(CONFIG_HARDENED_PAGE_MAPPINGS))
 		page = virt_to_page(addr);
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 06fed56..2fe0e98 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -12,6 +12,7 @@ config ARM64
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_HAS_GIGANTIC_PAGE
 	select ARCH_HAS_HARDENED_MAPPINGS
+	select ARCH_HAS_HARDENED_MODULE_MAPPINGS
 	select ARCH_HAS_KCOV
 	select ARCH_HAS_SG_CHAIN
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
index a26d27f..1eebe1f 100644
--- a/arch/arm64/Kconfig.debug
+++ b/arch/arm64/Kconfig.debug
@@ -71,17 +71,6 @@ config DEBUG_WX
 
 	  If in doubt, say "Y".
 
-config DEBUG_SET_MODULE_RONX
-	bool "Set loadable kernel module data as NX and text as RO"
-	depends on MODULES
-	default y
-	help
-	  Is this is set, kernel module text and rodata will be made read-only.
-	  This is to help catch accidental or malicious attempts to change the
-	  kernel's executable code.
-
-	  If in doubt, say Y.
-
 config DEBUG_ALIGN_RODATA
 	depends on ARCH_HAS_HARDENED_MAPPINGS
 	bool "Align linker sections up to SECTION_SIZE"
diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index 94b62c1..31bd53f 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -93,7 +93,7 @@ static void __kprobes *patch_map(void *addr, int fixmap)
 	bool module = !core_kernel_text(uintaddr);
 	struct page *page;
 
-	if (module && IS_ENABLED(CONFIG_DEBUG_SET_MODULE_RONX))
+	if (module && IS_ENABLED(CONFIG_HARDENED_MODULE_MAPPINGS))
 		page = vmalloc_to_page(addr);
 	else if (!module)
 		page = pfn_to_page(PHYS_PFN(__pa(addr)));
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 8e70ae5..b1e6ed5 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -69,6 +69,7 @@ config S390
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_HAS_GIGANTIC_PAGE
 	select ARCH_HAS_HARDENED_MAPPINGS
+	select ARCH_HAS_HARDENED_MODULE_MAPPINGS
 	select ARCH_HAS_KCOV
 	select ARCH_HAS_SG_CHAIN
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
diff --git a/arch/s390/Kconfig.debug b/arch/s390/Kconfig.debug
index 26c5d5be..57f8ea9 100644
--- a/arch/s390/Kconfig.debug
+++ b/arch/s390/Kconfig.debug
@@ -17,7 +17,4 @@ config S390_PTDUMP
 	  kernel.
 	  If in doubt, say "N"
 
-config DEBUG_SET_MODULE_RONX
-	def_bool y
-	depends on MODULES
 endmenu
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 9d80cd8..38ce850 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -51,6 +51,7 @@ config X86
 	select ARCH_HAS_FAST_MULTIPLIER
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_HAS_HARDENED_MAPPINGS
+	select ARCH_HAS_HARDENED_MODULE_MAPPINGS
 	select ARCH_HAS_KCOV			if X86_64
 	select ARCH_HAS_MMIO_FLUSH
 	select ARCH_HAS_PMEM_API		if X86_64
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 67eec55..69cdd0b 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -109,17 +109,6 @@ config DEBUG_WX
 
 	  If in doubt, say "Y".
 
-config DEBUG_SET_MODULE_RONX
-	bool "Set loadable kernel module data as NX and text as RO"
-	depends on MODULES
-	---help---
-	  This option helps catch unintended modifications to loadable
-	  kernel module's text and read-only data. It also prevents execution
-	  of module data. Such protection may interfere with run-time code
-	  patching and dynamic kernel tracing - and they might also protect
-	  against certain classes of kernel exploits.
-	  If in doubt, say "N".
-
 config DEBUG_NX_TEST
 	tristate "Testcase for the NX non-executable stack feature"
 	depends on DEBUG_KERNEL && m
diff --git a/include/linux/filter.h b/include/linux/filter.h
index e4eb254..5426940 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -545,7 +545,7 @@ static inline bool bpf_prog_was_classic(const struct bpf_prog *prog)
 
 #define bpf_classic_proglen(fprog) (fprog->len * sizeof(fprog->filter[0]))
 
-#ifdef CONFIG_DEBUG_SET_MODULE_RONX
+#ifdef CONFIG_HARDENED_MODULE_MAPPINGS
 static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
 {
 	set_memory_ro((unsigned long)fp, fp->pages);
@@ -563,7 +563,7 @@ static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
 static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
 {
 }
-#endif /* CONFIG_DEBUG_SET_MODULE_RONX */
+#endif /* CONFIG_HARDENED_MODULE_MAPPINGS */
 
 int sk_filter_trim_cap(struct sock *sk, struct sk_buff *skb, unsigned int cap);
 static inline int sk_filter(struct sock *sk, struct sk_buff *skb)
diff --git a/include/linux/init.h b/include/linux/init.h
index 9967bc9..5d6b0b2 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -126,7 +126,7 @@ void prepare_namespace(void);
 void __init load_default_modules(void);
 int __init init_rootfs(void);
 
-#if defined(CONFIG_HARDENED_PAGE_MAPPINGS) || defined(CONFIG_DEBUG_SET_MODULE_RONX)
+#if defined(CONFIG_HARDENED_PAGE_MAPPINGS) || defined(CONFIG_HARDENED_MODULE_MAPPINGS)
 extern bool rodata_enabled;
 #endif
 #ifdef CONFIG_HARDENED_PAGE_MAPPINGS
diff --git a/include/linux/module.h b/include/linux/module.h
index 7c84273..a4f6926 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -764,7 +764,7 @@ extern int module_sysfs_initialized;
 
 #define __MODULE_STRING(x) __stringify(x)
 
-#ifdef CONFIG_DEBUG_SET_MODULE_RONX
+#ifdef CONFIG_HARDENED_MODULE_MAPPINGS
 extern void set_all_modules_text_rw(void);
 extern void set_all_modules_text_ro(void);
 extern void module_enable_ro(const struct module *mod, bool after_init);
diff --git a/init/main.c b/init/main.c
index 4b3bcc4..1545399 100644
--- a/init/main.c
+++ b/init/main.c
@@ -925,7 +925,7 @@ static int try_to_run_init_process(const char *init_filename)
 
 static noinline void __init kernel_init_freeable(void);
 
-#if defined(CONFIG_HARDENED_PAGE_MAPPINGS) || defined(CONFIG_DEBUG_SET_MODULE_RONX)
+#if defined(CONFIG_HARDENED_PAGE_MAPPINGS) || defined(CONFIG_HARDENED_MODULE_MAPPINGS)
 bool rodata_enabled __ro_after_init = true;
 static int __init set_debug_rodata(char *str)
 {
diff --git a/kernel/module.c b/kernel/module.c
index 38d4270..eb2f865 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -74,9 +74,9 @@
 /*
  * Modules' sections will be aligned on page boundaries
  * to ensure complete separation of code and data, but
- * only when CONFIG_DEBUG_SET_MODULE_RONX=y
+ * only when CONFIG_HARDENED_MODULE_MAPPINGS=y
  */
-#ifdef CONFIG_DEBUG_SET_MODULE_RONX
+#ifdef CONFIG_HARDENED_MODULE_MAPPINGS
 # define debug_align(X) ALIGN(X, PAGE_SIZE)
 #else
 # define debug_align(X) (X)
@@ -1847,7 +1847,7 @@ static void mod_sysfs_teardown(struct module *mod)
 	mod_sysfs_fini(mod);
 }
 
-#ifdef CONFIG_DEBUG_SET_MODULE_RONX
+#ifdef CONFIG_HARDENED_MODULE_MAPPINGS
 /*
  * LKM RO/NX protection: protect module's text/ro-data
  * from modification and any data from execution.
diff --git a/security/Kconfig b/security/Kconfig
index ad6ce82..0f98d6b 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -174,6 +174,22 @@ config HARDENED_PAGE_MAPPINGS
 	  Unless your system has known restrictions or performance issues, it
 	  is recommended to say Y here.
 
+config ARCH_HAS_HARDENED_MODULE_MAPPINGS
+	def_bool n
+
+config HARDENED_MODULE_MAPPINGS
+	bool "Mark module mappings with stricter permissions (RO/W^X)"
+	default y
+	depends on ARCH_HAS_HARDENED_MODULE_MAPPINGS
+	help
+	  If this is set, module text and rodata memory will be made read-only,
+	  and non-text memory will be made non-executable. This provides
+	  protection against certain security vulnerabilities (e.g. modifying
+	  code)
+
+	  Unless your system has known restrictions or performance issues, it
+	  is recommended to say Y here.
+
 source security/selinux/Kconfig
 source security/smack/Kconfig
 source security/tomoyo/Kconfig
-- 
2.7.4

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

* Re: [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA
  2017-01-19  1:29 ` [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA Laura Abbott
@ 2017-01-19  7:53   ` Pavel Machek
  2017-01-25 11:21     ` Laura Abbott
  2017-01-19 10:56   ` Mark Rutland
  2017-01-19 21:57   ` Kees Cook
  2 siblings, 1 reply; 19+ messages in thread
From: Pavel Machek @ 2017-01-19  7:53 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Kees Cook, Jason Wessel, Jonathan Corbet, Russell King,
	Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Mark Rutland, Jessica Yu, linux-doc, linux-kernel,
	linux-arm-kernel, linux-parisc, linux-s390, linux-pm,
	kernel-hardening

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

On Wed 2017-01-18 17:29:05, Laura Abbott wrote:
> 
> Despite the word 'debug' in CONFIG_DEBUG_RODATA, this kernel option
> provides key security features that are to be expected on a modern
> system. Change the name to CONFIG_HARDENED_PAGE_MAPPINGS which more
> accurately describes what this option is intended to do.

I think this is bad change. CONFIG_DEBUG_RODATA is describing what it
does, CONFIG_HARDENED_PAGE_MAPPINGS is advertising.

We don't do advertising, and we don't force people to re-answer the
config questions without good reason.

CONFIG_HARDENED_RODATA might fix the first problem, but not the second
one.

								Pavel
								

> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
>  Documentation/DocBook/kgdb.tmpl            |  8 ++++----
>  Documentation/security/self-protection.txt |  2 +-
>  arch/arm/Kconfig                           |  1 +
>  arch/arm/configs/aspeed_g4_defconfig       |  2 +-
>  arch/arm/configs/aspeed_g5_defconfig       |  2 +-
>  arch/arm/include/asm/cacheflush.h          |  2 +-
>  arch/arm/kernel/patch.c                    |  2 +-
>  arch/arm/kernel/vmlinux.lds.S              |  8 ++++----
>  arch/arm/mm/Kconfig                        | 14 +-------------
>  arch/arm/mm/init.c                         |  4 ++--
>  arch/arm64/Kconfig                         |  4 +---
>  arch/arm64/Kconfig.debug                   |  2 +-
>  arch/parisc/Kconfig                        |  1 +
>  arch/parisc/Kconfig.debug                  | 11 -----------
>  arch/parisc/configs/712_defconfig          |  2 +-
>  arch/parisc/configs/c3000_defconfig        |  2 +-
>  arch/parisc/mm/init.c                      |  2 +-
>  arch/s390/Kconfig                          |  4 +---
>  arch/x86/Kconfig                           |  4 +---
>  include/linux/init.h                       |  4 ++--
>  init/main.c                                |  4 ++--
>  kernel/configs/android-recommended.config  |  2 +-
>  kernel/power/hibernate.c                   |  2 +-
>  kernel/power/power.h                       |  4 ++--
>  kernel/power/snapshot.c                    |  4 ++--
>  security/Kconfig                           | 16 ++++++++++++++++
>  26 files changed, 51 insertions(+), 62 deletions(-)
> 
> diff --git a/Documentation/DocBook/kgdb.tmpl b/Documentation/DocBook/kgdb.tmpl
> index f3abca7..a79b638 100644
> --- a/Documentation/DocBook/kgdb.tmpl
> +++ b/Documentation/DocBook/kgdb.tmpl
> @@ -115,12 +115,12 @@
>      </para>
>      <para>
>      If the architecture that you are using supports the kernel option
> -    CONFIG_DEBUG_RODATA, you should consider turning it off.  This
> +    CONFIG_HARDENED_PAGE_MAPPINGS, you should consider turning it off.  This
>      option will prevent the use of software breakpoints because it
>      marks certain regions of the kernel's memory space as read-only.
>      If kgdb supports it for the architecture you are using, you can
>      use hardware breakpoints if you desire to run with the
> -    CONFIG_DEBUG_RODATA option turned on, else you need to turn off
> +    CONFIG_HARDENED_PAGE_MAPPINGS option turned on, else you need to turn off
>      this option.
>      </para>
>      <para>
> @@ -135,7 +135,7 @@
>      <para>Here is an example set of .config symbols to enable or
>      disable for kgdb:
>      <itemizedlist>
> -    <listitem><para># CONFIG_DEBUG_RODATA is not set</para></listitem>
> +    <listitem><para># CONFIG_HARDENED_PAGE_MAPPINGS is not set</para></listitem>
>      <listitem><para>CONFIG_FRAME_POINTER=y</para></listitem>
>      <listitem><para>CONFIG_KGDB=y</para></listitem>
>      <listitem><para>CONFIG_KGDB_SERIAL_CONSOLE=y</para></listitem>
> @@ -166,7 +166,7 @@
>      </para>
>      <para>Here is an example set of .config symbols to enable/disable kdb:
>      <itemizedlist>
> -    <listitem><para># CONFIG_DEBUG_RODATA is not set</para></listitem>
> +    <listitem><para># CONFIG_HARDENED_PAGE_MAPPINGS is not set</para></listitem>
>      <listitem><para>CONFIG_FRAME_POINTER=y</para></listitem>
>      <listitem><para>CONFIG_KGDB=y</para></listitem>
>      <listitem><para>CONFIG_KGDB_SERIAL_CONSOLE=y</para></listitem>
> diff --git a/Documentation/security/self-protection.txt b/Documentation/security/self-protection.txt
> index 3010576..da8cb36 100644
> --- a/Documentation/security/self-protection.txt
> +++ b/Documentation/security/self-protection.txt
> @@ -51,7 +51,7 @@ kernel, they are implemented in a way where the memory is temporarily
>  made writable during the update, and then returned to the original
>  permissions.)
>  
> -In support of this are (the poorly named) CONFIG_DEBUG_RODATA and
> +In support of this are CONFIG_HARDENED_PAGE_MAPPINGS and
>  CONFIG_DEBUG_SET_MODULE_RONX, which seek to make sure that code is not
>  writable, data is not executable, and read-only data is neither writable
>  nor executable.
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 186c4c2..09aff28 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -7,6 +7,7 @@ config ARM
>  	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>  	select ARCH_HAVE_CUSTOM_GPIO_H
>  	select ARCH_HAS_GCOV_PROFILE_ALL
> +	select ARCH_HAS_HARDENED_MAPPINGS if MMU && !XIP_KERNEL
>  	select ARCH_MIGHT_HAVE_PC_PARPORT
>  	select ARCH_SUPPORTS_ATOMIC_RMW
>  	select ARCH_USE_BUILTIN_BSWAP
> diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig
> index ca39c04..8ccc216 100644
> --- a/arch/arm/configs/aspeed_g4_defconfig
> +++ b/arch/arm/configs/aspeed_g4_defconfig
> @@ -25,7 +25,7 @@ CONFIG_MODULE_UNLOAD=y
>  # CONFIG_ARCH_MULTI_V7 is not set
>  CONFIG_ARCH_ASPEED=y
>  CONFIG_MACH_ASPEED_G4=y
> -CONFIG_DEBUG_RODATA=y
> +CONFIG_HARDENED_PAGE_MAPPINGS=y
>  CONFIG_AEABI=y
>  CONFIG_UACCESS_WITH_MEMCPY=y
>  CONFIG_SECCOMP=y
> diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
> index 4f366b0..90c5ce4 100644
> --- a/arch/arm/configs/aspeed_g5_defconfig
> +++ b/arch/arm/configs/aspeed_g5_defconfig
> @@ -26,7 +26,7 @@ CONFIG_ARCH_MULTI_V6=y
>  # CONFIG_ARCH_MULTI_V7 is not set
>  CONFIG_ARCH_ASPEED=y
>  CONFIG_MACH_ASPEED_G5=y
> -CONFIG_DEBUG_RODATA=y
> +CONFIG_HARDENED_PAGE_MAPPINGS=y
>  CONFIG_AEABI=y
>  CONFIG_UACCESS_WITH_MEMCPY=y
>  CONFIG_SECCOMP=y
> diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
> index bdd283b..c3a7a72 100644
> --- a/arch/arm/include/asm/cacheflush.h
> +++ b/arch/arm/include/asm/cacheflush.h
> @@ -490,7 +490,7 @@ static inline int set_memory_x(unsigned long addr, int numpages) { return 0; }
>  static inline int set_memory_nx(unsigned long addr, int numpages) { return 0; }
>  #endif
>  
> -#ifdef CONFIG_DEBUG_RODATA
> +#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
>  void set_kernel_text_rw(void);
>  void set_kernel_text_ro(void);
>  #else
> diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
> index 69bda1a..9da1bf5 100644
> --- a/arch/arm/kernel/patch.c
> +++ b/arch/arm/kernel/patch.c
> @@ -26,7 +26,7 @@ static void __kprobes *patch_map(void *addr, int fixmap, unsigned long *flags)
>  
>  	if (module && IS_ENABLED(CONFIG_DEBUG_SET_MODULE_RONX))
>  		page = vmalloc_to_page(addr);
> -	else if (!module && IS_ENABLED(CONFIG_DEBUG_RODATA))
> +	else if (!module && IS_ENABLED(CONFIG_HARDENED_PAGE_MAPPINGS))
>  		page = virt_to_page(addr);
>  	else
>  		return addr;
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> index f7f55df..5c6a2e8 100644
> --- a/arch/arm/kernel/vmlinux.lds.S
> +++ b/arch/arm/kernel/vmlinux.lds.S
> @@ -97,7 +97,7 @@ SECTIONS
>  		HEAD_TEXT
>  	}
>  
> -#ifdef CONFIG_DEBUG_RODATA
> +#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
>  	. = ALIGN(1<<SECTION_SHIFT);
>  #endif
>  
> @@ -158,7 +158,7 @@ SECTIONS
>  
>  	NOTES
>  
> -#ifdef CONFIG_DEBUG_RODATA
> +#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
>  	. = ALIGN(1<<SECTION_SHIFT);
>  #else
>  	. = ALIGN(PAGE_SIZE);
> @@ -230,7 +230,7 @@ SECTIONS
>  	PERCPU_SECTION(L1_CACHE_BYTES)
>  #endif
>  
> -#ifdef CONFIG_DEBUG_RODATA
> +#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
>  	. = ALIGN(1<<SECTION_SHIFT);
>  #else
>  	. = ALIGN(THREAD_SIZE);
> @@ -325,7 +325,7 @@ SECTIONS
>  	STABS_DEBUG
>  }
>  
> -#ifdef CONFIG_DEBUG_RODATA
> +#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
>  /*
>   * Without CONFIG_DEBUG_ALIGN_RODATA, __start_rodata_section_aligned will
>   * be the first section-aligned location after __start_rodata. Otherwise,
> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
> index f68e8ec..e770dc9 100644
> --- a/arch/arm/mm/Kconfig
> +++ b/arch/arm/mm/Kconfig
> @@ -1051,21 +1051,9 @@ config ARCH_SUPPORTS_BIG_ENDIAN
>  	  This option specifies the architecture can support big endian
>  	  operation.
>  
> -config DEBUG_RODATA
> -	bool "Make kernel text and rodata read-only"
> -	depends on MMU && !XIP_KERNEL
> -	default y if CPU_V7
> -	help
> -	  If this is set, kernel text and rodata memory 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_ALIGN_RODATA
>  	bool "Make rodata strictly non-executable"
> -	depends on DEBUG_RODATA
> +	depends on HARDENED_PAGE_MAPPINGS
>  	default y
>  	help
>  	  If this is set, rodata will be made explicitly non-executable. This
> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> index 370581a..303bee4 100644
> --- a/arch/arm/mm/init.c
> +++ b/arch/arm/mm/init.c
> @@ -572,7 +572,7 @@ void __init mem_init(void)
>  	}
>  }
>  
> -#ifdef CONFIG_DEBUG_RODATA
> +#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
>  struct section_perm {
>  	const char *name;
>  	unsigned long start;
> @@ -741,7 +741,7 @@ void set_kernel_text_ro(void)
>  
>  #else
>  static inline void fix_kernmem_perms(void) { }
> -#endif /* CONFIG_DEBUG_RODATA */
> +#endif /* CONFIG_HARDENED_PAGE_MAPPINGS */
>  
>  void free_tcmmem(void)
>  {
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1117421..06fed56 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -11,6 +11,7 @@ config ARM64
>  	select ARCH_HAS_ELF_RANDOMIZE
>  	select ARCH_HAS_GCOV_PROFILE_ALL
>  	select ARCH_HAS_GIGANTIC_PAGE
> +	select ARCH_HAS_HARDENED_MAPPINGS
>  	select ARCH_HAS_KCOV
>  	select ARCH_HAS_SG_CHAIN
>  	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> @@ -123,9 +124,6 @@ config ARCH_PHYS_ADDR_T_64BIT
>  config MMU
>  	def_bool y
>  
> -config DEBUG_RODATA
> -	def_bool y
> -
>  config ARM64_PAGE_SHIFT
>  	int
>  	default 16 if ARM64_64K_PAGES
> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
> index d1ebd46..a26d27f 100644
> --- a/arch/arm64/Kconfig.debug
> +++ b/arch/arm64/Kconfig.debug
> @@ -83,7 +83,7 @@ config DEBUG_SET_MODULE_RONX
>  	  If in doubt, say Y.
>  
>  config DEBUG_ALIGN_RODATA
> -	depends on DEBUG_RODATA
> +	depends on ARCH_HAS_HARDENED_MAPPINGS
>  	bool "Align linker sections up to SECTION_SIZE"
>  	help
>  	  If this option is enabled, sections that may potentially be marked as
> diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
> index 3a71f38..7c73eaa 100644
> --- a/arch/parisc/Kconfig
> +++ b/arch/parisc/Kconfig
> @@ -8,6 +8,7 @@ config PARISC
>  	select HAVE_SYSCALL_TRACEPOINTS
>  	select ARCH_WANT_FRAME_POINTERS
>  	select ARCH_HAS_ELF_RANDOMIZE
> +	select ARCH_HAS_HARDENED_MAPPINGS
>  	select RTC_CLASS
>  	select RTC_DRV_GENERIC
>  	select INIT_ALL_POSSIBLE
> diff --git a/arch/parisc/Kconfig.debug b/arch/parisc/Kconfig.debug
> index 68b7cbd..0d856b9 100644
> --- a/arch/parisc/Kconfig.debug
> +++ b/arch/parisc/Kconfig.debug
> @@ -5,15 +5,4 @@ source "lib/Kconfig.debug"
>  config TRACE_IRQFLAGS_SUPPORT
>  	def_bool y
>  
> -config DEBUG_RODATA
> -       bool "Write protect kernel read-only data structures"
> -       depends on DEBUG_KERNEL
> -       default y
> -       help
> -         Mark the kernel read-only data as write-protected in the pagetables,
> -         in order to catch accidental (and incorrect) writes to such const
> -         data. This option may have a slight performance impact because a
> -         portion of the kernel code won't be covered by a TLB anymore.
> -         If in doubt, say "N".
> -
>  endmenu
> diff --git a/arch/parisc/configs/712_defconfig b/arch/parisc/configs/712_defconfig
> index db8f56b..f1a4732 100644
> --- a/arch/parisc/configs/712_defconfig
> +++ b/arch/parisc/configs/712_defconfig
> @@ -182,7 +182,7 @@ CONFIG_DEBUG_FS=y
>  CONFIG_DEBUG_KERNEL=y
>  CONFIG_DEBUG_MUTEXES=y
>  # CONFIG_RCU_CPU_STALL_DETECTOR is not set
> -CONFIG_DEBUG_RODATA=y
> +CONFIG_HARDENED_PAGE_MAPPINGS=y
>  CONFIG_CRYPTO_NULL=m
>  CONFIG_CRYPTO_TEST=m
>  CONFIG_CRYPTO_HMAC=y
> diff --git a/arch/parisc/configs/c3000_defconfig b/arch/parisc/configs/c3000_defconfig
> index fb92b89..03e88e2 100644
> --- a/arch/parisc/configs/c3000_defconfig
> +++ b/arch/parisc/configs/c3000_defconfig
> @@ -166,7 +166,7 @@ CONFIG_DEBUG_KERNEL=y
>  CONFIG_DEBUG_MUTEXES=y
>  # CONFIG_DEBUG_BUGVERBOSE is not set
>  # CONFIG_RCU_CPU_STALL_DETECTOR is not set
> -CONFIG_DEBUG_RODATA=y
> +CONFIG_HARDENED_PAGE_MAPPINGS=y
>  CONFIG_CRYPTO_NULL=m
>  CONFIG_CRYPTO_TEST=m
>  CONFIG_CRYPTO_MD5=m
> diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
> index e02ada3..f76f8ad 100644
> --- a/arch/parisc/mm/init.c
> +++ b/arch/parisc/mm/init.c
> @@ -545,7 +545,7 @@ void free_initmem(void)
>  }
>  
>  
> -#ifdef CONFIG_DEBUG_RODATA
> +#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
>  void mark_rodata_ro(void)
>  {
>  	/* rodata memory was already mapped with KERNEL_RO access rights by
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index c6722112..8e70ae5 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -62,15 +62,13 @@ config PCI_QUIRKS
>  config ARCH_SUPPORTS_UPROBES
>  	def_bool y
>  
> -config DEBUG_RODATA
> -	def_bool y
> -
>  config S390
>  	def_bool y
>  	select ARCH_HAS_DEVMEM_IS_ALLOWED
>  	select ARCH_HAS_ELF_RANDOMIZE
>  	select ARCH_HAS_GCOV_PROFILE_ALL
>  	select ARCH_HAS_GIGANTIC_PAGE
> +	select ARCH_HAS_HARDENED_MAPPINGS
>  	select ARCH_HAS_KCOV
>  	select ARCH_HAS_SG_CHAIN
>  	select ARCH_HAS_UBSAN_SANITIZE_ALL
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index e487493..9d80cd8 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -50,6 +50,7 @@ config X86
>  	select ARCH_HAS_ELF_RANDOMIZE
>  	select ARCH_HAS_FAST_MULTIPLIER
>  	select ARCH_HAS_GCOV_PROFILE_ALL
> +	select ARCH_HAS_HARDENED_MAPPINGS
>  	select ARCH_HAS_KCOV			if X86_64
>  	select ARCH_HAS_MMIO_FLUSH
>  	select ARCH_HAS_PMEM_API		if X86_64
> @@ -309,9 +310,6 @@ config ARCH_SUPPORTS_UPROBES
>  config FIX_EARLYCON_MEM
>  	def_bool y
>  
> -config DEBUG_RODATA
> -	def_bool y
> -
>  config PGTABLE_LEVELS
>  	int
>  	default 4 if X86_64
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 885c3e6..9967bc9 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -126,10 +126,10 @@ void prepare_namespace(void);
>  void __init load_default_modules(void);
>  int __init init_rootfs(void);
>  
> -#if defined(CONFIG_DEBUG_RODATA) || defined(CONFIG_DEBUG_SET_MODULE_RONX)
> +#if defined(CONFIG_HARDENED_PAGE_MAPPINGS) || defined(CONFIG_DEBUG_SET_MODULE_RONX)
>  extern bool rodata_enabled;
>  #endif
> -#ifdef CONFIG_DEBUG_RODATA
> +#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
>  void mark_rodata_ro(void);
>  #endif
>  
> diff --git a/init/main.c b/init/main.c
> index b0c9d6f..4b3bcc4 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -925,7 +925,7 @@ static int try_to_run_init_process(const char *init_filename)
>  
>  static noinline void __init kernel_init_freeable(void);
>  
> -#if defined(CONFIG_DEBUG_RODATA) || defined(CONFIG_DEBUG_SET_MODULE_RONX)
> +#if defined(CONFIG_HARDENED_PAGE_MAPPINGS) || defined(CONFIG_DEBUG_SET_MODULE_RONX)
>  bool rodata_enabled __ro_after_init = true;
>  static int __init set_debug_rodata(char *str)
>  {
> @@ -934,7 +934,7 @@ static int __init set_debug_rodata(char *str)
>  __setup("rodata=", set_debug_rodata);
>  #endif
>  
> -#ifdef CONFIG_DEBUG_RODATA
> +#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
>  static void mark_readonly(void)
>  {
>  	if (rodata_enabled)
> diff --git a/kernel/configs/android-recommended.config b/kernel/configs/android-recommended.config
> index 297756b..b796bc8 100644
> --- a/kernel/configs/android-recommended.config
> +++ b/kernel/configs/android-recommended.config
> @@ -11,7 +11,7 @@ CONFIG_BLK_DEV_LOOP=y
>  CONFIG_BLK_DEV_RAM=y
>  CONFIG_BLK_DEV_RAM_SIZE=8192
>  CONFIG_COMPACTION=y
> -CONFIG_DEBUG_RODATA=y
> +CONFIG_HARDENED_PAGE_MAPPINGS=y
>  CONFIG_DM_CRYPT=y
>  CONFIG_DM_UEVENT=y
>  CONFIG_DM_VERITY=y
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index b26dbc4..f7a3ea3 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -1156,7 +1156,7 @@ static int __init hibernate_setup(char *str)
>  	} else if (!strncmp(str, "no", 2)) {
>  		noresume = 1;
>  		nohibernate = 1;
> -	} else if (IS_ENABLED(CONFIG_DEBUG_RODATA)
> +	} else if (IS_ENABLED(CONFIG_HARDENED_PAGE_MAPPINGS)
>  		   && !strncmp(str, "protect_image", 13)) {
>  		enable_restore_image_protection();
>  	}
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 1dfa0da..dc2c7b8 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -61,12 +61,12 @@ extern int hibernation_snapshot(int platform_mode);
>  extern int hibernation_restore(int platform_mode);
>  extern int hibernation_platform_enter(void);
>  
> -#ifdef CONFIG_DEBUG_RODATA
> +#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
>  /* kernel/power/snapshot.c */
>  extern void enable_restore_image_protection(void);
>  #else
>  static inline void enable_restore_image_protection(void) {}
> -#endif /* CONFIG_DEBUG_RODATA */
> +#endif /* CONFIG_HARDENED_PAGE_MAPPINGS */
>  
>  #else /* !CONFIG_HIBERNATION */
>  
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index 2d8e2b2..a7c793e 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -38,7 +38,7 @@
>  
>  #include "power.h"
>  
> -#ifdef CONFIG_DEBUG_RODATA
> +#ifdef CONFIG_HARDENED_PAGE_MAPPINGS
>  static bool hibernate_restore_protection;
>  static bool hibernate_restore_protection_active;
>  
> @@ -73,7 +73,7 @@ static inline void hibernate_restore_protection_begin(void) {}
>  static inline void hibernate_restore_protection_end(void) {}
>  static inline void hibernate_restore_protect_page(void *page_address) {}
>  static inline void hibernate_restore_unprotect_page(void *page_address) {}
> -#endif /* CONFIG_DEBUG_RODATA */
> +#endif /* CONFIG_HARDENED_PAGE_MAPPINGS */
>  
>  static int swsusp_page_is_free(struct page *);
>  static void swsusp_set_page_forbidden(struct page *);
> diff --git a/security/Kconfig b/security/Kconfig
> index 118f454..ad6ce82 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -158,6 +158,22 @@ config HARDENED_USERCOPY_PAGESPAN
>  	  been removed. This config is intended to be used only while
>  	  trying to find such users.
>  
> +config ARCH_HAS_HARDENED_MAPPINGS
> +	def_bool n
> +
> +config HARDENED_PAGE_MAPPINGS
> +	bool "Mark kernel mappings with stricter permissions (RO/W^X)"
> +	default y
> +	depends on ARCH_HAS_HARDENED_MAPPINGS
> +	help
> +          If this is set, kernel text and rodata memory will be made read-only,
> +	  and non-text memory will be made non-executable. This provides
> +	  protection against certain security attacks (e.g. executing the heap
> +	  or modifying text).
> +
> +	  Unless your system has known restrictions or performance issues, it
> +	  is recommended to say Y here.
> +
>  source security/selinux/Kconfig
>  source security/smack/Kconfig
>  source security/tomoyo/Kconfig

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA
  2017-01-19  1:29 ` [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA Laura Abbott
  2017-01-19  7:53   ` Pavel Machek
@ 2017-01-19 10:56   ` Mark Rutland
  2017-01-19 11:33     ` Heiko Carstens
                       ` (2 more replies)
  2017-01-19 21:57   ` Kees Cook
  2 siblings, 3 replies; 19+ messages in thread
From: Mark Rutland @ 2017-01-19 10:56 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Kees Cook, Jason Wessel, Jonathan Corbet, Russell King,
	Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Pavel Machek, Jessica Yu, linux-doc, linux-kernel,
	linux-arm-kernel, linux-parisc, linux-s390, linux-pm,
	kernel-hardening

Hi Laura,

On Wed, Jan 18, 2017 at 05:29:05PM -0800, Laura Abbott wrote:
> 
> Despite the word 'debug' in CONFIG_DEBUG_RODATA, this kernel option
> provides key security features that are to be expected on a modern
> system. Change the name to CONFIG_HARDENED_PAGE_MAPPINGS which more
> accurately describes what this option is intended to do.

This generally sounds good. Thanks for attacking this!

On the bikeshedding front, *maybe* it would be nice to mention
permissions in the name, something like STRICT_KERNEL_RWX. That might
also prevent the reading of 'hardened' as 'optional overhead'.

That said, the proposed name is fine by me -- I'm happy so long as
'DEBUG' goes.

> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1117421..06fed56 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -11,6 +11,7 @@ config ARM64
>  	select ARCH_HAS_ELF_RANDOMIZE
>  	select ARCH_HAS_GCOV_PROFILE_ALL
>  	select ARCH_HAS_GIGANTIC_PAGE
> +	select ARCH_HAS_HARDENED_MAPPINGS
>  	select ARCH_HAS_KCOV
>  	select ARCH_HAS_SG_CHAIN
>  	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> @@ -123,9 +124,6 @@ config ARCH_PHYS_ADDR_T_64BIT
>  config MMU
>  	def_bool y
>  
> -config DEBUG_RODATA
> -	def_bool y
> -

> diff --git a/security/Kconfig b/security/Kconfig
> index 118f454..ad6ce82 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -158,6 +158,22 @@ config HARDENED_USERCOPY_PAGESPAN
>  	  been removed. This config is intended to be used only while
>  	  trying to find such users.
>  
> +config ARCH_HAS_HARDENED_MAPPINGS
> +	def_bool n
> +
> +config HARDENED_PAGE_MAPPINGS
> +	bool "Mark kernel mappings with stricter permissions (RO/W^X)"
> +	default y
> +	depends on ARCH_HAS_HARDENED_MAPPINGS
> +	help
> +          If this is set, kernel text and rodata memory will be made read-only,
> +	  and non-text memory will be made non-executable. This provides
> +	  protection against certain security attacks (e.g. executing the heap
> +	  or modifying text).
> +
> +	  Unless your system has known restrictions or performance issues, it
> +	  is recommended to say Y here.

It's somewhat unfortunate that this means the feature is no longer
mandatory on arm64 (and s390+x86). We have a boot-time switch to turn
the protections off, so I was hoping we could make this mandatory on all
architectures with support.

It would be good to see if we could make this mandatory for arm and
parisc, or if it really needs to be optional for either of those.

Thanks,
Mark.

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

* Re: [PATCH 2/2] security: Change name of CONFIG_DEBUG_SET_MODULE_RONX
  2017-01-19  1:29 ` [PATCH 2/2] security: Change name of CONFIG_DEBUG_SET_MODULE_RONX Laura Abbott
@ 2017-01-19 11:11   ` Mark Rutland
  2017-01-19 11:34     ` Heiko Carstens
  2017-01-19 11:43   ` Robin Murphy
  2017-01-20  5:46   ` kbuild test robot
  2 siblings, 1 reply; 19+ messages in thread
From: Mark Rutland @ 2017-01-19 11:11 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Kees Cook, Jason Wessel, Jonathan Corbet, Russell King,
	Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Pavel Machek, Jessica Yu, linux-doc, linux-kernel,
	linux-arm-kernel, linux-parisc, linux-s390, linux-pm,
	kernel-hardening, AKASHI, Takahiro

Hi,

On Wed, Jan 18, 2017 at 05:29:06PM -0800, Laura Abbott wrote:
> 
> Despite the word 'debug' in CONFIG_DEBUG_SET_MODULE_RONX, this kernel
> option provides key security features that are to be expected on a
> modern system. Change the name to CONFIG_HARDENED_MODULE_MAPPINGS which
> more accurately describes what this option is intended to do.

This looks good; my naming comments from the DEBUG_RODATA also apply
here -- the proposed name is fine.

> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 06fed56..2fe0e98 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -12,6 +12,7 @@ config ARM64
>  	select ARCH_HAS_GCOV_PROFILE_ALL
>  	select ARCH_HAS_GIGANTIC_PAGE
>  	select ARCH_HAS_HARDENED_MAPPINGS
> +	select ARCH_HAS_HARDENED_MODULE_MAPPINGS
>  	select ARCH_HAS_KCOV
>  	select ARCH_HAS_SG_CHAIN
>  	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
> index a26d27f..1eebe1f 100644
> --- a/arch/arm64/Kconfig.debug
> +++ b/arch/arm64/Kconfig.debug
> @@ -71,17 +71,6 @@ config DEBUG_WX
>  
>  	  If in doubt, say "Y".
>  
> -config DEBUG_SET_MODULE_RONX
> -	bool "Set loadable kernel module data as NX and text as RO"
> -	depends on MODULES
> -	default y
> -	help
> -	  Is this is set, kernel module text and rodata will be made read-only.
> -	  This is to help catch accidental or malicious attempts to change the
> -	  kernel's executable code.
> -
> -	  If in doubt, say Y.
> -

> +config ARCH_HAS_HARDENED_MODULE_MAPPINGS
> +	def_bool n
> +
> +config HARDENED_MODULE_MAPPINGS
> +	bool "Mark module mappings with stricter permissions (RO/W^X)"
> +	default y
> +	depends on ARCH_HAS_HARDENED_MODULE_MAPPINGS
> +	help
> +	  If this is set, module text and rodata memory will be made read-only,
> +	  and non-text memory will be made non-executable. This provides
> +	  protection against certain security vulnerabilities (e.g. modifying
> +	  code)
> +
> +	  Unless your system has known restrictions or performance issues, it
> +	  is recommended to say Y here.
> +

I was hoping that we'd make this mandatory, as we'd already done for
DEBUG_RODATA.

Takahiro-san did a bit of work towards that in commit 39290b389ea2654f
("module: extend 'rodata=off' boot cmdline parameter to module
mappings").

It would be good to know if there's any reason we can't do that.

Otherwise, this looks fine.

Thanks,
Mark.

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

* Re: [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA
  2017-01-19 10:56   ` Mark Rutland
@ 2017-01-19 11:33     ` Heiko Carstens
  2017-01-19 21:17       ` Helge Deller
  2017-01-25 11:37       ` Laura Abbott
  2017-01-19 22:00     ` Kees Cook
  2017-01-25 11:25     ` Laura Abbott
  2 siblings, 2 replies; 19+ messages in thread
From: Heiko Carstens @ 2017-01-19 11:33 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Laura Abbott, Kees Cook, Jason Wessel, Jonathan Corbet,
	Russell King, Catalin Marinas, Will Deacon, James E.J. Bottomley,
	Helge Deller, Martin Schwidefsky, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Pavel Machek, Jessica Yu, linux-doc, linux-kernel,
	linux-arm-kernel, linux-parisc, linux-s390, linux-pm,
	kernel-hardening

On Thu, Jan 19, 2017 at 10:56:46AM +0000, Mark Rutland wrote:
> > +config HARDENED_PAGE_MAPPINGS
> > +	bool "Mark kernel mappings with stricter permissions (RO/W^X)"
> > +	default y
> > +	depends on ARCH_HAS_HARDENED_MAPPINGS
> > +	help
> > +          If this is set, kernel text and rodata memory will be made read-only,
> > +	  and non-text memory will be made non-executable. This provides
> > +	  protection against certain security attacks (e.g. executing the heap
> > +	  or modifying text).
> > +
> > +	  Unless your system has known restrictions or performance issues, it
> > +	  is recommended to say Y here.
> 
> It's somewhat unfortunate that this means the feature is no longer
> mandatory on arm64 (and s390+x86). We have a boot-time switch to turn
> the protections off, so I was hoping we could make this mandatory on all
> architectures with support.
> 
> It would be good to see if we could make this mandatory for arm and
> parisc, or if it really needs to be optional for either of those.

Looks like the config option is a no-op on parisc just like it is on
s390. Irrelavant of the config option at least on s390 the page tables for
kernel text and rodata will be read-only anyway.

This works since ages and I don't see a reason why this should be
changed. Also trying to disable this with the "rodata=" command line option
does not work at least on s390, and I guess this is true for parisc as
well.

The only thing implemented with CONFIG_DEBUG_RODATA on both architectures
is to emit a message that states memory has been protected
(mark_rodata_ro).
This just avoids a wrong "Kernel memory protection disabled." message.

So yes, I'd really like to keep this option mandatory.

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

* Re: [PATCH 2/2] security: Change name of CONFIG_DEBUG_SET_MODULE_RONX
  2017-01-19 11:11   ` Mark Rutland
@ 2017-01-19 11:34     ` Heiko Carstens
  0 siblings, 0 replies; 19+ messages in thread
From: Heiko Carstens @ 2017-01-19 11:34 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Laura Abbott, Kees Cook, Jason Wessel, Jonathan Corbet,
	Russell King, Catalin Marinas, Will Deacon, James E.J. Bottomley,
	Helge Deller, Martin Schwidefsky, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Pavel Machek, Jessica Yu, linux-doc, linux-kernel,
	linux-arm-kernel, linux-parisc, linux-s390, linux-pm,
	kernel-hardening, AKASHI, Takahiro

On Thu, Jan 19, 2017 at 11:11:18AM +0000, Mark Rutland wrote:
> > +config HARDENED_MODULE_MAPPINGS
> > +	bool "Mark module mappings with stricter permissions (RO/W^X)"
> > +	default y
> > +	depends on ARCH_HAS_HARDENED_MODULE_MAPPINGS
> > +	help
> > +	  If this is set, module text and rodata memory will be made read-only,
> > +	  and non-text memory will be made non-executable. This provides
> > +	  protection against certain security vulnerabilities (e.g. modifying
> > +	  code)
> > +
> > +	  Unless your system has known restrictions or performance issues, it
> > +	  is recommended to say Y here.
> > +
> 
> I was hoping that we'd make this mandatory, as we'd already done for
> DEBUG_RODATA.

Same for s390: would be good to make this mandatory.

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

* Re: [PATCH 2/2] security: Change name of CONFIG_DEBUG_SET_MODULE_RONX
  2017-01-19  1:29 ` [PATCH 2/2] security: Change name of CONFIG_DEBUG_SET_MODULE_RONX Laura Abbott
  2017-01-19 11:11   ` Mark Rutland
@ 2017-01-19 11:43   ` Robin Murphy
  2017-01-25 11:44     ` Laura Abbott
  2017-01-20  5:46   ` kbuild test robot
  2 siblings, 1 reply; 19+ messages in thread
From: Robin Murphy @ 2017-01-19 11:43 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Kees Cook, Mark Rutland, linux-doc, Catalin Marinas,
	Heiko Carstens, James E.J. Bottomley, Pavel Machek,
	H. Peter Anvin, kernel-hardening, Rob Herring, Jessica Yu,
	Jonathan Corbet, Helge Deller, x86, Russell King, Ingo Molnar,
	Len Brown, linux-s390, Will Deacon, Thomas Gleixner,
	linux-arm-kernel, linux-parisc, linux-pm, Rafael J. Wysocki,
	linux-kernel, Jason Wessel, Martin Schwidefsky

Hi Laura,

On 19/01/17 01:29, Laura Abbott wrote:
> 
> Despite the word 'debug' in CONFIG_DEBUG_SET_MODULE_RONX, this kernel
> option provides key security features that are to be expected on a
> modern system. Change the name to CONFIG_HARDENED_MODULE_MAPPINGS which
> more accurately describes what this option is intended to do.
> 
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---

[...]

> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 09aff28..ef852e4 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -8,6 +8,7 @@ config ARM
>  	select ARCH_HAVE_CUSTOM_GPIO_H
>  	select ARCH_HAS_GCOV_PROFILE_ALL
>  	select ARCH_HAS_HARDENED_MAPPINGS if MMU && !XIP_KERNEL
> +	select ARCH_HAS_HARDENED_MODULE_MAPPINGS if MMU
>  	select ARCH_MIGHT_HAVE_PC_PARPORT
>  	select ARCH_SUPPORTS_ATOMIC_RMW
>  	select ARCH_USE_BUILTIN_BSWAP
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index d83f7c3..426d271 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -1738,17 +1738,6 @@ config PID_IN_CONTEXTIDR
>  	  additional instructions during context switch. Say Y here only if you
>  	  are planning to use hardware trace tools with this kernel.
>  
> -config DEBUG_SET_MODULE_RONX
> -	bool "Set loadable kernel module data as NX and text as RO"
> -	depends on MODULES && MMU
> -	---help---
> -	  This option helps catch unintended modifications to loadable
> -	  kernel module's text and read-only data. It also prevents execution
> -	  of module data. Such protection may interfere with run-time code
> -	  patching and dynamic kernel tracing - and they might also protect
> -	  against certain classes of kernel exploits.
> -	  If in doubt, say "N".
> -
>  source "drivers/hwtracing/coresight/Kconfig"
>  
>  endmenu

[...]

> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -12,6 +12,7 @@ config ARM64
>  	select ARCH_HAS_GCOV_PROFILE_ALL
>  	select ARCH_HAS_GIGANTIC_PAGE
>  	select ARCH_HAS_HARDENED_MAPPINGS
> +	select ARCH_HAS_HARDENED_MODULE_MAPPINGS
>  	select ARCH_HAS_KCOV
>  	select ARCH_HAS_SG_CHAIN
>  	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
> index a26d27f..1eebe1f 100644
> --- a/arch/arm64/Kconfig.debug
> +++ b/arch/arm64/Kconfig.debug
> @@ -71,17 +71,6 @@ config DEBUG_WX
>  
>  	  If in doubt, say "Y".
>  
> -config DEBUG_SET_MODULE_RONX
> -	bool "Set loadable kernel module data as NX and text as RO"
> -	depends on MODULES
> -	default y
> -	help
> -	  Is this is set, kernel module text and rodata will be made read-only.
> -	  This is to help catch accidental or malicious attempts to change the
> -	  kernel's executable code.
> -
> -	  If in doubt, say Y.
> -
>  config DEBUG_ALIGN_RODATA
>  	depends on ARCH_HAS_HARDENED_MAPPINGS
>  	bool "Align linker sections up to SECTION_SIZE"

[...]

> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -69,6 +69,7 @@ config S390
>  	select ARCH_HAS_GCOV_PROFILE_ALL
>  	select ARCH_HAS_GIGANTIC_PAGE
>  	select ARCH_HAS_HARDENED_MAPPINGS
> +	select ARCH_HAS_HARDENED_MODULE_MAPPINGS
>  	select ARCH_HAS_KCOV
>  	select ARCH_HAS_SG_CHAIN
>  	select ARCH_HAS_UBSAN_SANITIZE_ALL
> diff --git a/arch/s390/Kconfig.debug b/arch/s390/Kconfig.debug
> index 26c5d5be..57f8ea9 100644
> --- a/arch/s390/Kconfig.debug
> +++ b/arch/s390/Kconfig.debug
> @@ -17,7 +17,4 @@ config S390_PTDUMP
>  	  kernel.
>  	  If in doubt, say "N"
>  
> -config DEBUG_SET_MODULE_RONX
> -	def_bool y
> -	depends on MODULES
>  endmenu
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 9d80cd8..38ce850 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -51,6 +51,7 @@ config X86
>  	select ARCH_HAS_FAST_MULTIPLIER
>  	select ARCH_HAS_GCOV_PROFILE_ALL
>  	select ARCH_HAS_HARDENED_MAPPINGS
> +	select ARCH_HAS_HARDENED_MODULE_MAPPINGS
>  	select ARCH_HAS_KCOV			if X86_64
>  	select ARCH_HAS_MMIO_FLUSH
>  	select ARCH_HAS_PMEM_API		if X86_64
> diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
> index 67eec55..69cdd0b 100644
> --- a/arch/x86/Kconfig.debug
> +++ b/arch/x86/Kconfig.debug
> @@ -109,17 +109,6 @@ config DEBUG_WX
>  
>  	  If in doubt, say "Y".
>  
> -config DEBUG_SET_MODULE_RONX
> -	bool "Set loadable kernel module data as NX and text as RO"
> -	depends on MODULES
> -	---help---
> -	  This option helps catch unintended modifications to loadable
> -	  kernel module's text and read-only data. It also prevents execution
> -	  of module data. Such protection may interfere with run-time code
> -	  patching and dynamic kernel tracing - and they might also protect
> -	  against certain classes of kernel exploits.
> -	  If in doubt, say "N".
> -
>  config DEBUG_NX_TEST
>  	tristate "Testcase for the NX non-executable stack feature"
>  	depends on DEBUG_KERNEL && m

[...]

> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -174,6 +174,22 @@ config HARDENED_PAGE_MAPPINGS
>  	  Unless your system has known restrictions or performance issues, it
>  	  is recommended to say Y here.
>  
> +config ARCH_HAS_HARDENED_MODULE_MAPPINGS
> +	def_bool n
> +
> +config HARDENED_MODULE_MAPPINGS
> +	bool "Mark module mappings with stricter permissions (RO/W^X)"
> +	default y
> +	depends on ARCH_HAS_HARDENED_MODULE_MAPPINGS

It would seem that this ends up losing the previous dependency on
MODULES - is that intentional?

Robin.

> +	help
> +	  If this is set, module text and rodata memory will be made read-only,
> +	  and non-text memory will be made non-executable. This provides
> +	  protection against certain security vulnerabilities (e.g. modifying
> +	  code)
> +
> +	  Unless your system has known restrictions or performance issues, it
> +	  is recommended to say Y here.
> +
>  source security/selinux/Kconfig
>  source security/smack/Kconfig
>  source security/tomoyo/Kconfig
> 

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

* Re: [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA
  2017-01-19 11:33     ` Heiko Carstens
@ 2017-01-19 21:17       ` Helge Deller
  2017-01-25 11:37       ` Laura Abbott
  1 sibling, 0 replies; 19+ messages in thread
From: Helge Deller @ 2017-01-19 21:17 UTC (permalink / raw)
  To: Heiko Carstens, Mark Rutland
  Cc: Laura Abbott, Kees Cook, Jason Wessel, Jonathan Corbet,
	Russell King, Catalin Marinas, Will Deacon, James E.J. Bottomley,
	Martin Schwidefsky, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	x86, Rob Herring, Rafael J. Wysocki, Len Brown, Pavel Machek,
	Jessica Yu, linux-doc, linux-kernel, linux-arm-kernel,
	linux-parisc, linux-s390, linux-pm, kernel-hardening

On 19.01.2017 12:33, Heiko Carstens wrote:
> On Thu, Jan 19, 2017 at 10:56:46AM +0000, Mark Rutland wrote:
>>> +config HARDENED_PAGE_MAPPINGS
>>> +	bool "Mark kernel mappings with stricter permissions (RO/W^X)"
>>> +	default y
>>> +	depends on ARCH_HAS_HARDENED_MAPPINGS
>>> +	help
>>> +          If this is set, kernel text and rodata memory will be made read-only,
>>> +	  and non-text memory will be made non-executable. This provides
>>> +	  protection against certain security attacks (e.g. executing the heap
>>> +	  or modifying text).
>>> +
>>> +	  Unless your system has known restrictions or performance issues, it
>>> +	  is recommended to say Y here.
>>
>> It's somewhat unfortunate that this means the feature is no longer
>> mandatory on arm64 (and s390+x86). We have a boot-time switch to turn
>> the protections off, so I was hoping we could make this mandatory on all
>> architectures with support.
>>
>> It would be good to see if we could make this mandatory for arm and
>> parisc, or if it really needs to be optional for either of those.
> 
> Looks like the config option is a no-op on parisc just like it is on
> s390. Irrelavant of the config option at least on s390 the page tables for
> kernel text and rodata will be read-only anyway.

Right, that's true at the moment for parisc as well.
I do have unfinished patches which will add runtime kernel patching
for ftrace to parisc, and those patches will need to add code to 
enable/disable ro text/data like x86.
 
> This works since ages and I don't see a reason why this should be
> changed. Also trying to disable this with the "rodata=" command line option
> does not work at least on s390, and I guess this is true for parisc as
> well.

I never tried the option itself, but it should work on parisc to disable
the ro protection by this option.
 
> The only thing implemented with CONFIG_DEBUG_RODATA on both architectures
> is to emit a message that states memory has been protected
> (mark_rodata_ro).
> This just avoids a wrong "Kernel memory protection disabled." message.
> 
> So yes, I'd really like to keep this option mandatory.
 
I'd be fine with a rename of the config option to ARCH_HAS_HARDENED_MAPPINGS
and keeping the "rodata=" command line.

Helge

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

* Re: [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA
  2017-01-19  1:29 ` [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA Laura Abbott
  2017-01-19  7:53   ` Pavel Machek
  2017-01-19 10:56   ` Mark Rutland
@ 2017-01-19 21:57   ` Kees Cook
  2 siblings, 0 replies; 19+ messages in thread
From: Kees Cook @ 2017-01-19 21:57 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Jason Wessel, Jonathan Corbet, Russell King, Catalin Marinas,
	Will Deacon, James E.J. Bottomley, Helge Deller,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Pavel Machek, Mark Rutland, Jessica Yu, linux-doc, LKML,
	linux-arm-kernel, linux-parisc, linux-s390, Linux PM list,
	kernel-hardening

On Wed, Jan 18, 2017 at 5:29 PM, Laura Abbott <labbott@redhat.com> wrote:
>
> Despite the word 'debug' in CONFIG_DEBUG_RODATA, this kernel option
> provides key security features that are to be expected on a modern
> system. Change the name to CONFIG_HARDENED_PAGE_MAPPINGS which more
> accurately describes what this option is intended to do.

Oh thank you. Yes, this is badly needed. I might prefer to see this as
two patches, though:

Move DEBUG_RODATA to top-level arch/Kconfig, (and add ARCH_HAS_[bikeshed]).
Rename DEBUG_RODATA to [bikeshed]

(We should do a similar renaming for DEBUG_SET_MODULE_RONX too.)

Another thing that might be even cleaner would be to entirely invert
the logic. Something like CONFIG_ARCH_MISSING_[bikeshed]?

> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 186c4c2..09aff28 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -7,6 +7,7 @@ config ARM
>         select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>         select ARCH_HAVE_CUSTOM_GPIO_H
>         select ARCH_HAS_GCOV_PROFILE_ALL
> +       select ARCH_HAS_HARDENED_MAPPINGS if MMU && !XIP_KERNEL
>         select ARCH_MIGHT_HAVE_PC_PARPORT
>         select ARCH_SUPPORTS_ATOMIC_RMW
>         select ARCH_USE_BUILTIN_BSWAP
> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
> index f68e8ec..e770dc9 100644
> --- a/arch/arm/mm/Kconfig
> +++ b/arch/arm/mm/Kconfig
> @@ -1051,21 +1051,9 @@ config ARCH_SUPPORTS_BIG_ENDIAN
>           This option specifies the architecture can support big endian
>           operation.
>
> -config DEBUG_RODATA
> -       bool "Make kernel text and rodata read-only"
> -       depends on MMU && !XIP_KERNEL
> -       default y if CPU_V7

These changes aren't correctly representing the ARM state. I think the
ARCH_HAS is correct, but I'm not sure the best way to include the
"default y if CPU_V7".

> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -158,6 +158,22 @@ config HARDENED_USERCOPY_PAGESPAN
>           been removed. This config is intended to be used only while
>           trying to find such users.
>
> +config ARCH_HAS_HARDENED_MAPPINGS
> +       def_bool n
> +
> +config HARDENED_PAGE_MAPPINGS
> +       bool "Mark kernel mappings with stricter permissions (RO/W^X)"
> +       default y
> +       depends on ARCH_HAS_HARDENED_MAPPINGS
> +       help
> +          If this is set, kernel text and rodata memory will be made read-only,
> +         and non-text memory will be made non-executable. This provides
> +         protection against certain security attacks (e.g. executing the heap
> +         or modifying text).
> +
> +         Unless your system has known restrictions or performance issues, it
> +         is recommended to say Y here.
> +
>  source security/selinux/Kconfig
>  source security/smack/Kconfig
>  source security/tomoyo/Kconfig

Should this go in arch/Kconfig or security/Kconfig? I'm starting to
think we need a top-level kernel security Kconfig (the gcc plugins are
starting to pile up in arch/Kconfig, for example). I think since this
is arch specific, maybe arch/Kconfig? (Arguably, HARDENED_USERCOPY
shouldn't be in security/Kconfig either, since security/Kconfig is
mostly LSM or userspace-facing stuff? I dunno.)

As for the bikeshed on the naming, I like "KERNEL_RWX", and it likely
doesn't need "STRICT", IMO. CONFIG_KERNEL_RWX ? I don't have a strong
opinion beyond removing "DEBUG" from the name. :)

-Kees

-- 
Kees Cook
Nexus Security

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

* Re: [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA
  2017-01-19 10:56   ` Mark Rutland
  2017-01-19 11:33     ` Heiko Carstens
@ 2017-01-19 22:00     ` Kees Cook
  2017-01-25 11:25     ` Laura Abbott
  2 siblings, 0 replies; 19+ messages in thread
From: Kees Cook @ 2017-01-19 22:00 UTC (permalink / raw)
  To: Mark Rutland, Michael Ellerman
  Cc: Laura Abbott, Jason Wessel, Jonathan Corbet, Russell King,
	Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Pavel Machek, Jessica Yu, linux-doc, LKML, linux-arm-kernel,
	linux-parisc, linux-s390, Linux PM list, kernel-hardening

On Thu, Jan 19, 2017 at 2:56 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Laura,
>
> On Wed, Jan 18, 2017 at 05:29:05PM -0800, Laura Abbott wrote:
>> diff --git a/security/Kconfig b/security/Kconfig
>> index 118f454..ad6ce82 100644
>> --- a/security/Kconfig
>> +++ b/security/Kconfig
>> @@ -158,6 +158,22 @@ config HARDENED_USERCOPY_PAGESPAN
>>         been removed. This config is intended to be used only while
>>         trying to find such users.
>>
>> +config ARCH_HAS_HARDENED_MAPPINGS
>> +     def_bool n
>> +
>> +config HARDENED_PAGE_MAPPINGS
>> +     bool "Mark kernel mappings with stricter permissions (RO/W^X)"
>> +     default y
>> +     depends on ARCH_HAS_HARDENED_MAPPINGS
>> +     help
>> +          If this is set, kernel text and rodata memory will be made read-only,
>> +       and non-text memory will be made non-executable. This provides
>> +       protection against certain security attacks (e.g. executing the heap
>> +       or modifying text).
>> +
>> +       Unless your system has known restrictions or performance issues, it
>> +       is recommended to say Y here.
>
> It's somewhat unfortunate that this means the feature is no longer
> mandatory on arm64 (and s390+x86). We have a boot-time switch to turn
> the protections off, so I was hoping we could make this mandatory on all
> architectures with support.

Oh, I totally missed this. Yes, we need it to stay mandatory. It
should be possible by just adding "select HARDENED_PAGE_MAPPINGS" to
the arch Kconfig, yes?

> It would be good to see if we could make this mandatory for arm and
> parisc, or if it really needs to be optional for either of those.

(Adding mpe to CC...) Michael, what's needed to get this working on powerpc too?

-Kees

-- 
Kees Cook
Nexus Security

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

* Re: [RFC][PATCH 0/2] Better hardening names
  2017-01-19  1:29 [RFC][PATCH 0/2] Better hardening names Laura Abbott
  2017-01-19  1:29 ` [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA Laura Abbott
  2017-01-19  1:29 ` [PATCH 2/2] security: Change name of CONFIG_DEBUG_SET_MODULE_RONX Laura Abbott
@ 2017-01-19 22:08 ` Kees Cook
  2 siblings, 0 replies; 19+ messages in thread
From: Kees Cook @ 2017-01-19 22:08 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Jason Wessel, Jonathan Corbet, Russell King, Catalin Marinas,
	Will Deacon, James E.J. Bottomley, Helge Deller,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Pavel Machek, Mark Rutland, Jessica Yu, linux-doc, LKML,
	linux-arm-kernel, linux-parisc, linux-s390, Linux PM list,
	kernel-hardening

On Wed, Jan 18, 2017 at 5:29 PM, Laura Abbott <labbott@redhat.com> wrote:
> Hi,
>
> It's come up previously that CONFIG_DEBUG_SET_MODULE_RONX and
> CONFIG_DEBUG_RODATA are not accurate names, mostly they should not have the
> word 'debug' in them. This series attempts to change the names to something
> a bit more descriptive and indicative of what they are actually used for these
> days.
>
> I marked this RFC for
> - Bike shedding purposes.
> - A discussion of what defaults should be. The way I did the refactoring, both
>   options are default y. I'd appreciate comments if there is a better approach.
> - Approach to split this up into more sub patches to make review/merging easier?
>   Or maybe it's fine.
>
> Quickly tested on arm/arm64/x86.
>
> Thanks,
> Laura
>
> Laura Abbott (2):
>   security: Change name of CONFIG_DEBUG_RODATA
>   security: Change name of CONFIG_DEBUG_SET_MODULE_RONX

Some day I'll quit reading my email backwards. :) I see you've got
MODULE_RONX renamed too here, please ignore my comment about needing
it "too". :)

-Kees

-- 
Kees Cook
Nexus Security

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

* Re: [PATCH 2/2] security: Change name of CONFIG_DEBUG_SET_MODULE_RONX
  2017-01-19  1:29 ` [PATCH 2/2] security: Change name of CONFIG_DEBUG_SET_MODULE_RONX Laura Abbott
  2017-01-19 11:11   ` Mark Rutland
  2017-01-19 11:43   ` Robin Murphy
@ 2017-01-20  5:46   ` kbuild test robot
  2 siblings, 0 replies; 19+ messages in thread
From: kbuild test robot @ 2017-01-20  5:46 UTC (permalink / raw)
  To: Laura Abbott
  Cc: kbuild-all, Kees Cook, Laura Abbott, Jason Wessel,
	Jonathan Corbet, Russell King, Catalin Marinas, Will Deacon,
	James E.J. Bottomley, Helge Deller, Martin Schwidefsky,
	Heiko Carstens, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	x86, Rob Herring, Rafael J. Wysocki, Len Brown, Pavel Machek,
	Mark Rutland, Jessica Yu, linux-doc, linux-kernel,
	linux-arm-kernel, linux-parisc, linux-s390, linux-pm,
	kernel-hardening

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

Hi Laura,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.10-rc4]
[cannot apply to next-20170119]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Laura-Abbott/Better-hardening-names/20170119-200343
config: i386-randconfig-c0-01201130 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86/built-in.o: In function `ftrace_arch_code_modify_prepare':
>> (.text+0x3fcb7): undefined reference to `set_all_modules_text_rw'
   arch/x86/built-in.o: In function `ftrace_arch_code_modify_post_process':
>> (.text+0x3fcc3): undefined reference to `set_all_modules_text_ro'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29640 bytes --]

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

* Re: [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA
  2017-01-19  7:53   ` Pavel Machek
@ 2017-01-25 11:21     ` Laura Abbott
  2017-01-25 13:51       ` Pavel Machek
  0 siblings, 1 reply; 19+ messages in thread
From: Laura Abbott @ 2017-01-25 11:21 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Kees Cook, Jason Wessel, Jonathan Corbet, Russell King,
	Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Mark Rutland, Jessica Yu, linux-doc, linux-kernel,
	linux-arm-kernel, linux-parisc, linux-s390, linux-pm,
	kernel-hardening

On 01/19/2017 08:53 AM, Pavel Machek wrote:
> On Wed 2017-01-18 17:29:05, Laura Abbott wrote:
>>
>> Despite the word 'debug' in CONFIG_DEBUG_RODATA, this kernel option
>> provides key security features that are to be expected on a modern
>> system. Change the name to CONFIG_HARDENED_PAGE_MAPPINGS which more
>> accurately describes what this option is intended to do.
>
> I think this is bad change. CONFIG_DEBUG_RODATA is describing what it
> does, CONFIG_HARDENED_PAGE_MAPPINGS is advertising.
>
> We don't do advertising, and we don't force people to re-answer the
> config questions without good reason.
>
> CONFIG_HARDENED_RODATA might fix the first problem, but not the second
> one.
>
> 								Pavel
> 								

(Apologies, my SMTP was set up incorrectly so my response didn't
actually get sent out)

CONFIG_DEBUG_RODATA isn't describing what it does though. It misses
that this config may handle much more than just rodata. I think
Mark Rutland's suggestion of STRICT_KERNEL_RWX might be more
descriptive.

Thanks,
Laura

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

* Re: [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA
  2017-01-19 10:56   ` Mark Rutland
  2017-01-19 11:33     ` Heiko Carstens
  2017-01-19 22:00     ` Kees Cook
@ 2017-01-25 11:25     ` Laura Abbott
  2 siblings, 0 replies; 19+ messages in thread
From: Laura Abbott @ 2017-01-25 11:25 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Kees Cook, Jason Wessel, Jonathan Corbet, Russell King,
	Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Pavel Machek, Jessica Yu, linux-doc, linux-kernel,
	linux-arm-kernel, linux-parisc, linux-s390, linux-pm,
	kernel-hardening

On 01/19/2017 11:56 AM, Mark Rutland wrote:
> Hi Laura,
>
> On Wed, Jan 18, 2017 at 05:29:05PM -0800, Laura Abbott wrote:
>>
>> Despite the word 'debug' in CONFIG_DEBUG_RODATA, this kernel option
>> provides key security features that are to be expected on a modern
>> system. Change the name to CONFIG_HARDENED_PAGE_MAPPINGS which more
>> accurately describes what this option is intended to do.
>
> This generally sounds good. Thanks for attacking this!
>
> On the bikeshedding front, *maybe* it would be nice to mention
> permissions in the name, something like STRICT_KERNEL_RWX. That might
> also prevent the reading of 'hardened' as 'optional overhead'.
>
> That said, the proposed name is fine by me -- I'm happy so long as
> 'DEBUG' goes.
>

(Apologies for the delay, my SMTP was set up incorrectly so my
messages didn't actually get sent out)

I like that better since it's describing specifically what the config
should be setting as opposed to something more vague. That might fit
better with what Pavel was suggesting as well.

Thanks,
Laura

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

* Re: [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA
  2017-01-19 11:33     ` Heiko Carstens
  2017-01-19 21:17       ` Helge Deller
@ 2017-01-25 11:37       ` Laura Abbott
  1 sibling, 0 replies; 19+ messages in thread
From: Laura Abbott @ 2017-01-25 11:37 UTC (permalink / raw)
  To: Heiko Carstens, Mark Rutland
  Cc: Kees Cook, Jason Wessel, Jonathan Corbet, Russell King,
	Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller,
	Martin Schwidefsky, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	x86, Rob Herring, Rafael J. Wysocki, Len Brown, Pavel Machek,
	Jessica Yu, linux-doc, linux-kernel, linux-arm-kernel,
	linux-parisc, linux-s390, linux-pm, kernel-hardening

On 01/19/2017 12:33 PM, Heiko Carstens wrote:
> On Thu, Jan 19, 2017 at 10:56:46AM +0000, Mark Rutland wrote:
>>> +config HARDENED_PAGE_MAPPINGS
>>> +	bool "Mark kernel mappings with stricter permissions (RO/W^X)"
>>> +	default y
>>> +	depends on ARCH_HAS_HARDENED_MAPPINGS
>>> +	help
>>> +          If this is set, kernel text and rodata memory will be made read-only,
>>> +	  and non-text memory will be made non-executable. This provides
>>> +	  protection against certain security attacks (e.g. executing the heap
>>> +	  or modifying text).
>>> +
>>> +	  Unless your system has known restrictions or performance issues, it
>>> +	  is recommended to say Y here.
>>
>> It's somewhat unfortunate that this means the feature is no longer
>> mandatory on arm64 (and s390+x86). We have a boot-time switch to turn
>> the protections off, so I was hoping we could make this mandatory on all
>> architectures with support.
>>
>> It would be good to see if we could make this mandatory for arm and
>> parisc, or if it really needs to be optional for either of those.
>
> Looks like the config option is a no-op on parisc just like it is on
> s390. Irrelavant of the config option at least on s390 the page tables for
> kernel text and rodata will be read-only anyway.
>
> This works since ages and I don't see a reason why this should be
> changed. Also trying to disable this with the "rodata=" command line option
> does not work at least on s390, and I guess this is true for parisc as
> well.
>
> The only thing implemented with CONFIG_DEBUG_RODATA on both architectures
> is to emit a message that states memory has been protected
> (mark_rodata_ro).
> This just avoids a wrong "Kernel memory protection disabled." message.
>
> So yes, I'd really like to keep this option mandatory.
>

(Apologies, my SMTP server was set up incorrectly so this didn't get
sent out when I thought it did)

Okay, that's useful to know. I think I'm going to add a
'select HARDENED_MAPPINGS' (or whatever it gets changed to) to arches
that were previously def_bool. This is a slight Kconfig semantic change
but as has been pointed out we now have the command line option.

Thanks,
Laura

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

* Re: [PATCH 2/2] security: Change name of CONFIG_DEBUG_SET_MODULE_RONX
  2017-01-19 11:43   ` Robin Murphy
@ 2017-01-25 11:44     ` Laura Abbott
  0 siblings, 0 replies; 19+ messages in thread
From: Laura Abbott @ 2017-01-25 11:44 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Kees Cook, Mark Rutland, linux-doc, Catalin Marinas,
	Heiko Carstens, James E.J. Bottomley, Pavel Machek,
	H. Peter Anvin, kernel-hardening, Rob Herring, Jessica Yu,
	Jonathan Corbet, Helge Deller, x86, Russell King, Ingo Molnar,
	Len Brown, linux-s390, Will Deacon, Thomas Gleixner,
	linux-arm-kernel, linux-parisc, linux-pm, Rafael J. Wysocki,
	linux-kernel, Jason Wessel, Martin Schwidefsky

On 01/19/2017 12:43 PM, Robin Murphy wrote:
> Hi Laura,
>
> On 19/01/17 01:29, Laura Abbott wrote:
>>
>> Despite the word 'debug' in CONFIG_DEBUG_SET_MODULE_RONX, this kernel
>> option provides key security features that are to be expected on a
>> modern system. Change the name to CONFIG_HARDENED_MODULE_MAPPINGS which
>> more accurately describes what this option is intended to do.
>>
>> Signed-off-by: Laura Abbott <labbott@redhat.com>
>> ---
>
> [...]
>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 09aff28..ef852e4 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -8,6 +8,7 @@ config ARM
>>  	select ARCH_HAVE_CUSTOM_GPIO_H
>>  	select ARCH_HAS_GCOV_PROFILE_ALL
>>  	select ARCH_HAS_HARDENED_MAPPINGS if MMU && !XIP_KERNEL
>> +	select ARCH_HAS_HARDENED_MODULE_MAPPINGS if MMU
>>  	select ARCH_MIGHT_HAVE_PC_PARPORT
>>  	select ARCH_SUPPORTS_ATOMIC_RMW
>>  	select ARCH_USE_BUILTIN_BSWAP
>> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
>> index d83f7c3..426d271 100644
>> --- a/arch/arm/Kconfig.debug
>> +++ b/arch/arm/Kconfig.debug
>> @@ -1738,17 +1738,6 @@ config PID_IN_CONTEXTIDR
>>  	  additional instructions during context switch. Say Y here only if you
>>  	  are planning to use hardware trace tools with this kernel.
>>
>> -config DEBUG_SET_MODULE_RONX
>> -	bool "Set loadable kernel module data as NX and text as RO"
>> -	depends on MODULES && MMU
>> -	---help---
>> -	  This option helps catch unintended modifications to loadable
>> -	  kernel module's text and read-only data. It also prevents execution
>> -	  of module data. Such protection may interfere with run-time code
>> -	  patching and dynamic kernel tracing - and they might also protect
>> -	  against certain classes of kernel exploits.
>> -	  If in doubt, say "N".
>> -
>>  source "drivers/hwtracing/coresight/Kconfig"
>>
>>  endmenu
>
> [...]
>
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -12,6 +12,7 @@ config ARM64
>>  	select ARCH_HAS_GCOV_PROFILE_ALL
>>  	select ARCH_HAS_GIGANTIC_PAGE
>>  	select ARCH_HAS_HARDENED_MAPPINGS
>> +	select ARCH_HAS_HARDENED_MODULE_MAPPINGS
>>  	select ARCH_HAS_KCOV
>>  	select ARCH_HAS_SG_CHAIN
>>  	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
>> index a26d27f..1eebe1f 100644
>> --- a/arch/arm64/Kconfig.debug
>> +++ b/arch/arm64/Kconfig.debug
>> @@ -71,17 +71,6 @@ config DEBUG_WX
>>
>>  	  If in doubt, say "Y".
>>
>> -config DEBUG_SET_MODULE_RONX
>> -	bool "Set loadable kernel module data as NX and text as RO"
>> -	depends on MODULES
>> -	default y
>> -	help
>> -	  Is this is set, kernel module text and rodata will be made read-only.
>> -	  This is to help catch accidental or malicious attempts to change the
>> -	  kernel's executable code.
>> -
>> -	  If in doubt, say Y.
>> -
>>  config DEBUG_ALIGN_RODATA
>>  	depends on ARCH_HAS_HARDENED_MAPPINGS
>>  	bool "Align linker sections up to SECTION_SIZE"
>
> [...]
>
>> --- a/arch/s390/Kconfig
>> +++ b/arch/s390/Kconfig
>> @@ -69,6 +69,7 @@ config S390
>>  	select ARCH_HAS_GCOV_PROFILE_ALL
>>  	select ARCH_HAS_GIGANTIC_PAGE
>>  	select ARCH_HAS_HARDENED_MAPPINGS
>> +	select ARCH_HAS_HARDENED_MODULE_MAPPINGS
>>  	select ARCH_HAS_KCOV
>>  	select ARCH_HAS_SG_CHAIN
>>  	select ARCH_HAS_UBSAN_SANITIZE_ALL
>> diff --git a/arch/s390/Kconfig.debug b/arch/s390/Kconfig.debug
>> index 26c5d5be..57f8ea9 100644
>> --- a/arch/s390/Kconfig.debug
>> +++ b/arch/s390/Kconfig.debug
>> @@ -17,7 +17,4 @@ config S390_PTDUMP
>>  	  kernel.
>>  	  If in doubt, say "N"
>>
>> -config DEBUG_SET_MODULE_RONX
>> -	def_bool y
>> -	depends on MODULES
>>  endmenu
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 9d80cd8..38ce850 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -51,6 +51,7 @@ config X86
>>  	select ARCH_HAS_FAST_MULTIPLIER
>>  	select ARCH_HAS_GCOV_PROFILE_ALL
>>  	select ARCH_HAS_HARDENED_MAPPINGS
>> +	select ARCH_HAS_HARDENED_MODULE_MAPPINGS
>>  	select ARCH_HAS_KCOV			if X86_64
>>  	select ARCH_HAS_MMIO_FLUSH
>>  	select ARCH_HAS_PMEM_API		if X86_64
>> diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
>> index 67eec55..69cdd0b 100644
>> --- a/arch/x86/Kconfig.debug
>> +++ b/arch/x86/Kconfig.debug
>> @@ -109,17 +109,6 @@ config DEBUG_WX
>>
>>  	  If in doubt, say "Y".
>>
>> -config DEBUG_SET_MODULE_RONX
>> -	bool "Set loadable kernel module data as NX and text as RO"
>> -	depends on MODULES
>> -	---help---
>> -	  This option helps catch unintended modifications to loadable
>> -	  kernel module's text and read-only data. It also prevents execution
>> -	  of module data. Such protection may interfere with run-time code
>> -	  patching and dynamic kernel tracing - and they might also protect
>> -	  against certain classes of kernel exploits.
>> -	  If in doubt, say "N".
>> -
>>  config DEBUG_NX_TEST
>>  	tristate "Testcase for the NX non-executable stack feature"
>>  	depends on DEBUG_KERNEL && m
>
> [...]
>
>> --- a/security/Kconfig
>> +++ b/security/Kconfig
>> @@ -174,6 +174,22 @@ config HARDENED_PAGE_MAPPINGS
>>  	  Unless your system has known restrictions or performance issues, it
>>  	  is recommended to say Y here.
>>
>> +config ARCH_HAS_HARDENED_MODULE_MAPPINGS
>> +	def_bool n
>> +
>> +config HARDENED_MODULE_MAPPINGS
>> +	bool "Mark module mappings with stricter permissions (RO/W^X)"
>> +	default y
>> +	depends on ARCH_HAS_HARDENED_MODULE_MAPPINGS
>
> It would seem that this ends up losing the previous dependency on
> MODULES - is that intentional?
>
> Robin.
>

(Apologies, my SMTP was set up incorrectly so this didn't actually
get sent out when I thought it did)

No, good catch. I missed re-adding that when doing the refactoring.

Thanks,
Laura

>>
>

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

* Re: [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA
  2017-01-25 11:21     ` Laura Abbott
@ 2017-01-25 13:51       ` Pavel Machek
  0 siblings, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2017-01-25 13:51 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Kees Cook, Jason Wessel, Jonathan Corbet, Russell King,
	Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Rob Herring, Rafael J. Wysocki, Len Brown,
	Mark Rutland, Jessica Yu, linux-doc, linux-kernel,
	linux-arm-kernel, linux-parisc, linux-s390, linux-pm,
	kernel-hardening

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

On Wed 2017-01-25 12:21:05, Laura Abbott wrote:
> On 01/19/2017 08:53 AM, Pavel Machek wrote:
> >On Wed 2017-01-18 17:29:05, Laura Abbott wrote:
> >>
> >>Despite the word 'debug' in CONFIG_DEBUG_RODATA, this kernel option
> >>provides key security features that are to be expected on a modern
> >>system. Change the name to CONFIG_HARDENED_PAGE_MAPPINGS which more
> >>accurately describes what this option is intended to do.
> >
> >I think this is bad change. CONFIG_DEBUG_RODATA is describing what it
> >does, CONFIG_HARDENED_PAGE_MAPPINGS is advertising.
> >
> >We don't do advertising, and we don't force people to re-answer the
> >config questions without good reason.
> >
> >CONFIG_HARDENED_RODATA might fix the first problem, but not the second
> >one.
> >
> >								Pavel
> >								
> 
> (Apologies, my SMTP was set up incorrectly so my response didn't
> actually get sent out)
> 
> CONFIG_DEBUG_RODATA isn't describing what it does though. It misses
> that this config may handle much more than just rodata. I think
> Mark Rutland's suggestion of STRICT_KERNEL_RWX might be more
> descriptive.

CONFIG_BUG=y
CONFIG_LBDAF=y
CONFIG_PM_OPP=y

..it is config option. It is not description of the feature. People
are living with that config option for a while. I'd keep it.

Maybe you can go from CONFIG_DEBUG_RODATA to CONFIG_RODATA... (but
you'll still have people re-answer config option.)


								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

end of thread, other threads:[~2017-01-25 13:51 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-19  1:29 [RFC][PATCH 0/2] Better hardening names Laura Abbott
2017-01-19  1:29 ` [PATCH 1/2] security: Change name of CONFIG_DEBUG_RODATA Laura Abbott
2017-01-19  7:53   ` Pavel Machek
2017-01-25 11:21     ` Laura Abbott
2017-01-25 13:51       ` Pavel Machek
2017-01-19 10:56   ` Mark Rutland
2017-01-19 11:33     ` Heiko Carstens
2017-01-19 21:17       ` Helge Deller
2017-01-25 11:37       ` Laura Abbott
2017-01-19 22:00     ` Kees Cook
2017-01-25 11:25     ` Laura Abbott
2017-01-19 21:57   ` Kees Cook
2017-01-19  1:29 ` [PATCH 2/2] security: Change name of CONFIG_DEBUG_SET_MODULE_RONX Laura Abbott
2017-01-19 11:11   ` Mark Rutland
2017-01-19 11:34     ` Heiko Carstens
2017-01-19 11:43   ` Robin Murphy
2017-01-25 11:44     ` Laura Abbott
2017-01-20  5:46   ` kbuild test robot
2017-01-19 22:08 ` [RFC][PATCH 0/2] Better hardening names 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).