* [PATCHv2 0/2] Hardening configs refactor/rename @ 2017-02-03 17:52 Laura Abbott 2017-02-03 17:52 ` [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common Laura Abbott 2017-02-03 17:52 ` [PATCHv2 2/2] arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX Laura Abbott 0 siblings, 2 replies; 15+ messages in thread From: Laura Abbott @ 2017-02-03 17:52 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-ker Hi, This is a follow up to my proposal to rename/refactor CONFIG_DEBUG_RODATA and CONFIG_DEBUG_SET_MODULE_RONX. Among other objections, there shouldn't be 'debug' in the name since these provide necessary kernel protection. v2 takes a slightly different approach to this per feedback. Patch #1 moves CONFIG_DEBUG_RODATA and CONFIG_DEBUG_SET_MODULE_RONX to a common arch config. These configs are def_bool y for every arch except !CPU_V7 for arm CONFIG_DEBUG_RODATA. I think this also mitigates another concern about changing the name since these are basically internal configs at this point and not end user selectable. Patch #2 does the rename to something more descriptive. Hopefully this should separate discussion more clearly into two parts (refactor and rename) Thanks, Laura Laura Abbott (2): arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX Documentation/DocBook/kgdb.tmpl | 8 ++++---- Documentation/security/self-protection.txt | 4 ++-- arch/Kconfig | 28 ++++++++++++++++++++++++++++ arch/arm/Kconfig | 3 +++ arch/arm/Kconfig.debug | 11 ----------- arch/arm/configs/aspeed_g4_defconfig | 3 +-- arch/arm/configs/aspeed_g5_defconfig | 3 +-- 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 | 1 - arch/parisc/configs/c3000_defconfig | 1 - 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 ++-- 33 files changed, 75 insertions(+), 109 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common 2017-02-03 17:52 [PATCHv2 0/2] Hardening configs refactor/rename Laura Abbott @ 2017-02-03 17:52 ` Laura Abbott 2017-02-03 18:16 ` Mark Rutland 2017-02-03 19:45 ` Kees Cook 2017-02-03 17:52 ` [PATCHv2 2/2] arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX Laura Abbott 1 sibling, 2 replies; 15+ messages in thread From: Laura Abbott @ 2017-02-03 17:52 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 There are multiple architectures that support CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX. These options also now have the ability to be turned off at runtime. Move these to an architecture independent location and make these options def_bool y for almost all of those arches. Signed-off-by: Laura Abbott <labbott@redhat.com> --- v2: This patch is now doing just the refactor of the existing config options. --- arch/Kconfig | 28 ++++++++++++++++++++++++++++ arch/arm/Kconfig | 3 +++ arch/arm/Kconfig.debug | 11 ----------- arch/arm/mm/Kconfig | 12 ------------ arch/arm64/Kconfig | 5 ++--- arch/arm64/Kconfig.debug | 11 ----------- arch/parisc/Kconfig | 1 + arch/parisc/Kconfig.debug | 11 ----------- arch/s390/Kconfig | 5 ++--- arch/s390/Kconfig.debug | 3 --- arch/x86/Kconfig | 5 ++--- arch/x86/Kconfig.debug | 11 ----------- 12 files changed, 38 insertions(+), 68 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index 99839c2..22ee01e 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -781,4 +781,32 @@ config VMAP_STACK the stack to map directly to the KASAN shadow map using a formula that is incorrect if the stack is in vmalloc space. +config ARCH_NO_STRICT_RWX_DEFAULTS + def_bool n + +config ARCH_HAS_STRICT_KERNEL_RWX + def_bool n + +config DEBUG_RODATA + def_bool y if !ARCH_NO_STRICT_RWX_DEFAULTS + prompt "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS + depends on ARCH_HAS_STRICT_KERNEL_RWX + 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 exploits (e.g. executing the heap + or modifying text) + +config ARCH_HAS_STRICT_MODULE_RWX + def_bool n + +config DEBUG_SET_MODULE_RONX + def_bool y if !ARCH_NO_STRICT_RWX_DEFAULTS + prompt "Set loadable kenrel module data as NX and text as RO" if ARCH_NO_STRICT_RWX_DEFAULTS + depends on ARCH_HAS_STRICT_MODULE_RWX && MODULES + 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 exploits (e.g. writing to text) + source "kernel/gcov/Kconfig" diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 186c4c2..aa73ca8 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -4,10 +4,13 @@ config ARM select ARCH_CLOCKSOURCE_DATA select ARCH_HAS_DEVMEM_IS_ALLOWED select ARCH_HAS_ELF_RANDOMIZE + select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL + select ARCH_HAS_STRICT_MODULE_RWX if MMU select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_HAVE_CUSTOM_GPIO_H select ARCH_HAS_GCOV_PROFILE_ALL select ARCH_MIGHT_HAVE_PC_PARPORT + select ARCH_NO_STRICT_RWX_DEFAULTS if !CPU_V7 select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_USE_BUILTIN_BSWAP select ARCH_USE_CMPXCHG_LOCKREF 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/mm/Kconfig b/arch/arm/mm/Kconfig index f68e8ec..419a035 100644 --- a/arch/arm/mm/Kconfig +++ b/arch/arm/mm/Kconfig @@ -1051,18 +1051,6 @@ 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 diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 1117421..e1efbcc 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -13,6 +13,8 @@ config ARM64 select ARCH_HAS_GIGANTIC_PAGE select ARCH_HAS_KCOV select ARCH_HAS_SG_CHAIN + select ARCH_HAS_STRICT_KERNEL_RWX + select ARCH_HAS_STRICT_MODULE_RWX select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_USE_CMPXCHG_LOCKREF select ARCH_SUPPORTS_ATOMIC_RMW @@ -123,9 +125,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..939815e 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 DEBUG_RODATA bool "Align linker sections up to SECTION_SIZE" diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig index 3a71f38..ad294b3 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_STRICT_KERNEL_RWX 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/s390/Kconfig b/arch/s390/Kconfig index c6722112..53bb0e3 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -62,9 +62,6 @@ 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 @@ -73,6 +70,8 @@ config S390 select ARCH_HAS_GIGANTIC_PAGE select ARCH_HAS_KCOV select ARCH_HAS_SG_CHAIN + select ARCH_HAS_STRICT_KERNEL_RWX + select ARCH_HAS_STRICT_MODULE_RWX select ARCH_HAS_UBSAN_SANITIZE_ALL select ARCH_HAVE_NMI_SAFE_CMPXCHG select ARCH_INLINE_READ_LOCK 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 e487493..13e1bf4 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -54,6 +54,8 @@ config X86 select ARCH_HAS_MMIO_FLUSH select ARCH_HAS_PMEM_API if X86_64 select ARCH_HAS_SG_CHAIN + select ARCH_HAS_STRICT_KERNEL_RWX + select ARCH_HAS_STRICT_MODULE_RWX select ARCH_HAS_UBSAN_SANITIZE_ALL select ARCH_HAVE_NMI_SAFE_CMPXCHG select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI @@ -309,9 +311,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/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 -- 2.7.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common 2017-02-03 17:52 ` [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common Laura Abbott @ 2017-02-03 18:16 ` Mark Rutland 2017-02-03 19:45 ` Kees Cook 1 sibling, 0 replies; 15+ messages in thread From: Mark Rutland @ 2017-02-03 18:16 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 On Fri, Feb 03, 2017 at 09:52:21AM -0800, Laura Abbott wrote: > There are multiple architectures that support CONFIG_DEBUG_RODATA and > CONFIG_SET_MODULE_RONX. These options also now have the ability to be > turned off at runtime. Move these to an architecture independent > location and make these options def_bool y for almost all of those > arches. > > Signed-off-by: Laura Abbott <labbott@redhat.com> >From my POV this looks good. FWIW: Acked-by: Mark Rutland <mark.rutland@arm.com> Mark. > --- > v2: This patch is now doing just the refactor of the existing config options. > --- > arch/Kconfig | 28 ++++++++++++++++++++++++++++ > arch/arm/Kconfig | 3 +++ > arch/arm/Kconfig.debug | 11 ----------- > arch/arm/mm/Kconfig | 12 ------------ > arch/arm64/Kconfig | 5 ++--- > arch/arm64/Kconfig.debug | 11 ----------- > arch/parisc/Kconfig | 1 + > arch/parisc/Kconfig.debug | 11 ----------- > arch/s390/Kconfig | 5 ++--- > arch/s390/Kconfig.debug | 3 --- > arch/x86/Kconfig | 5 ++--- > arch/x86/Kconfig.debug | 11 ----------- > 12 files changed, 38 insertions(+), 68 deletions(-) > > diff --git a/arch/Kconfig b/arch/Kconfig > index 99839c2..22ee01e 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -781,4 +781,32 @@ config VMAP_STACK > the stack to map directly to the KASAN shadow map using a formula > that is incorrect if the stack is in vmalloc space. > > +config ARCH_NO_STRICT_RWX_DEFAULTS > + def_bool n > + > +config ARCH_HAS_STRICT_KERNEL_RWX > + def_bool n > + > +config DEBUG_RODATA > + def_bool y if !ARCH_NO_STRICT_RWX_DEFAULTS > + prompt "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS > + depends on ARCH_HAS_STRICT_KERNEL_RWX > + 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 exploits (e.g. executing the heap > + or modifying text) > + > +config ARCH_HAS_STRICT_MODULE_RWX > + def_bool n > + > +config DEBUG_SET_MODULE_RONX > + def_bool y if !ARCH_NO_STRICT_RWX_DEFAULTS > + prompt "Set loadable kenrel module data as NX and text as RO" if ARCH_NO_STRICT_RWX_DEFAULTS > + depends on ARCH_HAS_STRICT_MODULE_RWX && MODULES > + 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 exploits (e.g. writing to text) > + > source "kernel/gcov/Kconfig" > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 186c4c2..aa73ca8 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -4,10 +4,13 @@ config ARM > select ARCH_CLOCKSOURCE_DATA > select ARCH_HAS_DEVMEM_IS_ALLOWED > select ARCH_HAS_ELF_RANDOMIZE > + select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL > + select ARCH_HAS_STRICT_MODULE_RWX if MMU > select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST > select ARCH_HAVE_CUSTOM_GPIO_H > select ARCH_HAS_GCOV_PROFILE_ALL > select ARCH_MIGHT_HAVE_PC_PARPORT > + select ARCH_NO_STRICT_RWX_DEFAULTS if !CPU_V7 > select ARCH_SUPPORTS_ATOMIC_RMW > select ARCH_USE_BUILTIN_BSWAP > select ARCH_USE_CMPXCHG_LOCKREF > 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/mm/Kconfig b/arch/arm/mm/Kconfig > index f68e8ec..419a035 100644 > --- a/arch/arm/mm/Kconfig > +++ b/arch/arm/mm/Kconfig > @@ -1051,18 +1051,6 @@ 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 > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 1117421..e1efbcc 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -13,6 +13,8 @@ config ARM64 > select ARCH_HAS_GIGANTIC_PAGE > select ARCH_HAS_KCOV > select ARCH_HAS_SG_CHAIN > + select ARCH_HAS_STRICT_KERNEL_RWX > + select ARCH_HAS_STRICT_MODULE_RWX > select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST > select ARCH_USE_CMPXCHG_LOCKREF > select ARCH_SUPPORTS_ATOMIC_RMW > @@ -123,9 +125,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..939815e 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 DEBUG_RODATA > bool "Align linker sections up to SECTION_SIZE" > diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig > index 3a71f38..ad294b3 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_STRICT_KERNEL_RWX > 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/s390/Kconfig b/arch/s390/Kconfig > index c6722112..53bb0e3 100644 > --- a/arch/s390/Kconfig > +++ b/arch/s390/Kconfig > @@ -62,9 +62,6 @@ 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 > @@ -73,6 +70,8 @@ config S390 > select ARCH_HAS_GIGANTIC_PAGE > select ARCH_HAS_KCOV > select ARCH_HAS_SG_CHAIN > + select ARCH_HAS_STRICT_KERNEL_RWX > + select ARCH_HAS_STRICT_MODULE_RWX > select ARCH_HAS_UBSAN_SANITIZE_ALL > select ARCH_HAVE_NMI_SAFE_CMPXCHG > select ARCH_INLINE_READ_LOCK > 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 e487493..13e1bf4 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -54,6 +54,8 @@ config X86 > select ARCH_HAS_MMIO_FLUSH > select ARCH_HAS_PMEM_API if X86_64 > select ARCH_HAS_SG_CHAIN > + select ARCH_HAS_STRICT_KERNEL_RWX > + select ARCH_HAS_STRICT_MODULE_RWX > select ARCH_HAS_UBSAN_SANITIZE_ALL > select ARCH_HAVE_NMI_SAFE_CMPXCHG > select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI > @@ -309,9 +311,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/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 > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common 2017-02-03 17:52 ` [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common Laura Abbott 2017-02-03 18:16 ` Mark Rutland @ 2017-02-03 19:45 ` Kees Cook 2017-02-03 20:29 ` Russell King - ARM Linux 1 sibling, 1 reply; 15+ messages in thread From: Kees Cook @ 2017-02-03 19:45 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@kernel.org, Rob Herring, Rafael J. Wysocki, Len Brown, Pavel Machek, Mark Rutland, Jessica Yu, linux-doc@vger.kernel.org On Fri, Feb 3, 2017 at 9:52 AM, Laura Abbott <labbott@redhat.com> wrote: > There are multiple architectures that support CONFIG_DEBUG_RODATA and > CONFIG_SET_MODULE_RONX. These options also now have the ability to be > turned off at runtime. Move these to an architecture independent > location and make these options def_bool y for almost all of those > arches. > > Signed-off-by: Laura Abbott <labbott@redhat.com> > --- > v2: This patch is now doing just the refactor of the existing config options. > --- > arch/Kconfig | 28 ++++++++++++++++++++++++++++ > arch/arm/Kconfig | 3 +++ > arch/arm/Kconfig.debug | 11 ----------- > arch/arm/mm/Kconfig | 12 ------------ > arch/arm64/Kconfig | 5 ++--- > arch/arm64/Kconfig.debug | 11 ----------- > arch/parisc/Kconfig | 1 + > arch/parisc/Kconfig.debug | 11 ----------- > arch/s390/Kconfig | 5 ++--- > arch/s390/Kconfig.debug | 3 --- > arch/x86/Kconfig | 5 ++--- > arch/x86/Kconfig.debug | 11 ----------- > 12 files changed, 38 insertions(+), 68 deletions(-) > > diff --git a/arch/Kconfig b/arch/Kconfig > index 99839c2..22ee01e 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -781,4 +781,32 @@ config VMAP_STACK > the stack to map directly to the KASAN shadow map using a formula > that is incorrect if the stack is in vmalloc space. > > +config ARCH_NO_STRICT_RWX_DEFAULTS > + def_bool n > + > +config ARCH_HAS_STRICT_KERNEL_RWX > + def_bool n > + > +config DEBUG_RODATA > + def_bool y if !ARCH_NO_STRICT_RWX_DEFAULTS > + prompt "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS Ah! Yes, perfect. I totally forgot about using conditional "prompt" lines. Nice! Acked-by: Kees Cook <keescook@chromium.org> -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common 2017-02-03 19:45 ` Kees Cook @ 2017-02-03 20:29 ` Russell King - ARM Linux 2017-02-03 21:08 ` Kees Cook 0 siblings, 1 reply; 15+ messages in thread From: Russell King - ARM Linux @ 2017-02-03 20:29 UTC (permalink / raw) To: Kees Cook Cc: Laura Abbott, Jason Wessel, Jonathan Corbet, Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller, Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86@kernel.org, Rob Herring, Rafael J. Wysocki, Len Brown, Pavel Machek, Mark Rutland, Jessica Yu, linux-doc@vger.kernel.org, LKML, linux-arm-kernel@lists.infradead.org, linux-pa On Fri, Feb 03, 2017 at 11:45:56AM -0800, Kees Cook wrote: > On Fri, Feb 3, 2017 at 9:52 AM, Laura Abbott <labbott@redhat.com> wrote: > > diff --git a/arch/Kconfig b/arch/Kconfig > > index 99839c2..22ee01e 100644 > > --- a/arch/Kconfig > > +++ b/arch/Kconfig > > @@ -781,4 +781,32 @@ config VMAP_STACK > > the stack to map directly to the KASAN shadow map using a formula > > that is incorrect if the stack is in vmalloc space. > > > > +config ARCH_NO_STRICT_RWX_DEFAULTS > > + def_bool n > > + > > +config ARCH_HAS_STRICT_KERNEL_RWX > > + def_bool n > > + > > +config DEBUG_RODATA > > + def_bool y if !ARCH_NO_STRICT_RWX_DEFAULTS > > + prompt "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS > > Ah! Yes, perfect. I totally forgot about using conditional "prompt" > lines. Nice! It's no different from the more usual: bool "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS default y if !ARCH_NO_STRICT_RWX_DEFAULTS depends on ARCH_HAS_STRICT_KERNEL_RWX But... I really don't like this - way too many negations and negatives which make it difficult to figure out what's going on here. The situation we have today is: -config DEBUG_RODATA - bool "Make kernel text and rodata read-only" - depends on MMU && !XIP_KERNEL - default y if CPU_V7 which is "allow the user to select DEBUG_RODATA if building a MMU non-XIP kernel", suggesting that the user turns it on for ARMv7 CPUs. That changes with this and the above: + select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL + select ARCH_HAS_STRICT_MODULE_RWX if MMU + select ARCH_NO_STRICT_RWX_DEFAULTS if !CPU_V7 This means that ARCH_HAS_STRICT_KERNEL_RWX is set for a MMU non-XIP kernel, which carries the same pre-condition for DEBUG_RODATA - no problem there. However, ARCH_NO_STRICT_RWX_DEFAULTS is set for non-ARMv7 CPUs, which means the "Make kernel text and rodata read-only" prompt _is_ provided for those. However, for all ARMv7 systems, we go from "suggesting that the user enables the option" to "you don't have a choice, you get this whether you want it or not." I'd prefer to keep it off for my development systems, where I don't care about kernel security. If we don't wish to do that as a general rule, can we make it dependent on EMBEDDED? Given that on ARM it can add up to 4MB to the kernel image - there _will_ be about 1MB before the .text section, the padding on between __modver and __ex_table which for me is around 626k, the padding between .notes and the init sections start with .vectors (the space between __ex_table and end of .notes is only 4124, which gets padded up to 1MB) and lastly the padding between the .init section and the data section (for me around 593k). This all adds up to an increase in kernel image size of 3.2MB on 14.2MB - an increase of 22%. So no, I'm really not happy with that. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common 2017-02-03 20:29 ` Russell King - ARM Linux @ 2017-02-03 21:08 ` Kees Cook 2017-02-03 22:28 ` Russell King - ARM Linux 2017-02-06 18:47 ` Laura Abbott 0 siblings, 2 replies; 15+ messages in thread From: Kees Cook @ 2017-02-03 21:08 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Laura Abbott, Jason Wessel, Jonathan Corbet, Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller, Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86@kernel.org, Rob Herring, Rafael J. Wysocki, Len Brown, Pavel Machek, Mark Rutland, Jessica Yu, linux-doc@vger.kernel.org On Fri, Feb 3, 2017 at 12:29 PM, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Fri, Feb 03, 2017 at 11:45:56AM -0800, Kees Cook wrote: >> On Fri, Feb 3, 2017 at 9:52 AM, Laura Abbott <labbott@redhat.com> wrote: >> > diff --git a/arch/Kconfig b/arch/Kconfig >> > index 99839c2..22ee01e 100644 >> > --- a/arch/Kconfig >> > +++ b/arch/Kconfig >> > @@ -781,4 +781,32 @@ config VMAP_STACK >> > the stack to map directly to the KASAN shadow map using a formula >> > that is incorrect if the stack is in vmalloc space. >> > >> > +config ARCH_NO_STRICT_RWX_DEFAULTS >> > + def_bool n >> > + >> > +config ARCH_HAS_STRICT_KERNEL_RWX >> > + def_bool n >> > + >> > +config DEBUG_RODATA >> > + def_bool y if !ARCH_NO_STRICT_RWX_DEFAULTS >> > + prompt "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS >> >> Ah! Yes, perfect. I totally forgot about using conditional "prompt" >> lines. Nice! > > It's no different from the more usual: > > bool "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS > default y if !ARCH_NO_STRICT_RWX_DEFAULTS > depends on ARCH_HAS_STRICT_KERNEL_RWX > > But... I really don't like this - way too many negations and negatives > which make it difficult to figure out what's going on here. > > The situation we have today is: > > -config DEBUG_RODATA > - bool "Make kernel text and rodata read-only" > - depends on MMU && !XIP_KERNEL > - default y if CPU_V7 > > which is "allow the user to select DEBUG_RODATA if building a MMU non-XIP > kernel", suggesting that the user turns it on for ARMv7 CPUs. > > That changes with this and the above: > > + select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL > + select ARCH_HAS_STRICT_MODULE_RWX if MMU > + select ARCH_NO_STRICT_RWX_DEFAULTS if !CPU_V7 > > This means that ARCH_HAS_STRICT_KERNEL_RWX is set for a MMU non-XIP > kernel, which carries the same pre-condition for DEBUG_RODATA - no > problem there. > > However, ARCH_NO_STRICT_RWX_DEFAULTS is set for non-ARMv7 CPUs, which > means the "Make kernel text and rodata read-only" prompt _is_ provided > for those. However, for all ARMv7 systems, we go from "suggesting that > the user enables the option" to "you don't have a choice, you get this > whether you want it or not." > > I'd prefer to keep it off for my development systems, where I don't > care about kernel security. If we don't wish to do that as a general > rule, can we make it dependent on EMBEDDED? > > Given that on ARM it can add up to 4MB to the kernel image - there > _will_ be about 1MB before the .text section, the padding on between > __modver and __ex_table which for me is around 626k, the padding > between .notes and the init sections start with .vectors (the space > between __ex_table and end of .notes is only 4124, which gets padded > up to 1MB) and lastly the padding between the .init section and the > data section (for me around 593k). This all adds up to an increase > in kernel image size of 3.2MB on 14.2MB - an increase of 22%. > > So no, I'm really not happy with that. Ah yeah, good point. We have three cases: unsupported, mandatory, optional, but we have the case of setting the default for the optional case. Maybe something like this? config STRICT_KERNEL_RWX bool "Make kernel text and rodata read-only" if ARCH_OPTIONAL_KERNEL_RWX depends on ARCH_HAS_STRICT_KERNEL_RWX default ARCH_OPTIONAL_KERNEL_RWX_DEFAULT unsupported: !ARCH_HAS_STRICT_KERNEL_RWX mandatory: ARCH_HAS_STRICT_KERNEL_RWX !ARCH_OPTIONAL_KERNEL_RWX optional: ARCH_HAS_STRICT_KERNEL_RWX ARCH_OPTIONAL_KERNEL_RWX with default controlled by ARCH_OPTIONAL_KERNEL_RWX_DEFAULT Then arm is: select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL select ARCH_HAS_STRICT_MODULE_RWX if MMU select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT if CPU_V7 x86 and arm64 are: select ARCH_HAS_STRICT_KERNEL_RWX select ARCH_HAS_STRICT_MODULE_RWX ? -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common 2017-02-03 21:08 ` Kees Cook @ 2017-02-03 22:28 ` Russell King - ARM Linux 2017-02-03 23:07 ` Kees Cook 2017-02-06 18:47 ` Laura Abbott 1 sibling, 1 reply; 15+ messages in thread From: Russell King - ARM Linux @ 2017-02-03 22:28 UTC (permalink / raw) To: Kees Cook Cc: Laura Abbott, Jason Wessel, Jonathan Corbet, Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller, Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86@kernel.org, Rob Herring, Rafael J. Wysocki, Len Brown, Pavel Machek, Mark Rutland, Jessica Yu, linux-doc@vger.kernel.org On Fri, Feb 03, 2017 at 01:08:40PM -0800, Kees Cook wrote: > On Fri, Feb 3, 2017 at 12:29 PM, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: > > On Fri, Feb 03, 2017 at 11:45:56AM -0800, Kees Cook wrote: > >> On Fri, Feb 3, 2017 at 9:52 AM, Laura Abbott <labbott@redhat.com> wrote: > >> > diff --git a/arch/Kconfig b/arch/Kconfig > >> > index 99839c2..22ee01e 100644 > >> > --- a/arch/Kconfig > >> > +++ b/arch/Kconfig > >> > @@ -781,4 +781,32 @@ config VMAP_STACK > >> > the stack to map directly to the KASAN shadow map using a formula > >> > that is incorrect if the stack is in vmalloc space. > >> > > >> > +config ARCH_NO_STRICT_RWX_DEFAULTS > >> > + def_bool n > >> > + > >> > +config ARCH_HAS_STRICT_KERNEL_RWX > >> > + def_bool n > >> > + > >> > +config DEBUG_RODATA > >> > + def_bool y if !ARCH_NO_STRICT_RWX_DEFAULTS > >> > + prompt "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS > >> > >> Ah! Yes, perfect. I totally forgot about using conditional "prompt" > >> lines. Nice! > > > > It's no different from the more usual: > > > > bool "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS > > default y if !ARCH_NO_STRICT_RWX_DEFAULTS > > depends on ARCH_HAS_STRICT_KERNEL_RWX > > > > But... I really don't like this - way too many negations and negatives > > which make it difficult to figure out what's going on here. > > > > The situation we have today is: > > > > -config DEBUG_RODATA > > - bool "Make kernel text and rodata read-only" > > - depends on MMU && !XIP_KERNEL > > - default y if CPU_V7 > > > > which is "allow the user to select DEBUG_RODATA if building a MMU non-XIP > > kernel", suggesting that the user turns it on for ARMv7 CPUs. > > > > That changes with this and the above: > > > > + select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL > > + select ARCH_HAS_STRICT_MODULE_RWX if MMU > > + select ARCH_NO_STRICT_RWX_DEFAULTS if !CPU_V7 > > > > This means that ARCH_HAS_STRICT_KERNEL_RWX is set for a MMU non-XIP > > kernel, which carries the same pre-condition for DEBUG_RODATA - no > > problem there. > > > > However, ARCH_NO_STRICT_RWX_DEFAULTS is set for non-ARMv7 CPUs, which > > means the "Make kernel text and rodata read-only" prompt _is_ provided > > for those. However, for all ARMv7 systems, we go from "suggesting that > > the user enables the option" to "you don't have a choice, you get this > > whether you want it or not." > > > > I'd prefer to keep it off for my development systems, where I don't > > care about kernel security. If we don't wish to do that as a general > > rule, can we make it dependent on EMBEDDED? > > > > Given that on ARM it can add up to 4MB to the kernel image - there > > _will_ be about 1MB before the .text section, the padding on between > > __modver and __ex_table which for me is around 626k, the padding > > between .notes and the init sections start with .vectors (the space > > between __ex_table and end of .notes is only 4124, which gets padded > > up to 1MB) and lastly the padding between the .init section and the > > data section (for me around 593k). This all adds up to an increase > > in kernel image size of 3.2MB on 14.2MB - an increase of 22%. > > > > So no, I'm really not happy with that. > > Ah yeah, good point. We have three cases: unsupported, mandatory, > optional, but we have the case of setting the default for the optional > case. Maybe something like this? > > config STRICT_KERNEL_RWX > bool "Make kernel text and rodata read-only" if ARCH_OPTIONAL_KERNEL_RWX > depends on ARCH_HAS_STRICT_KERNEL_RWX > default ARCH_OPTIONAL_KERNEL_RWX_DEFAULT > > unsupported: > !ARCH_HAS_STRICT_KERNEL_RWX > > mandatory: > ARCH_HAS_STRICT_KERNEL_RWX > !ARCH_OPTIONAL_KERNEL_RWX > > optional: > ARCH_HAS_STRICT_KERNEL_RWX > ARCH_OPTIONAL_KERNEL_RWX > with default controlled by ARCH_OPTIONAL_KERNEL_RWX_DEFAULT > > Then arm is: > select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL > select ARCH_HAS_STRICT_MODULE_RWX if MMU > select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX > select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT if CPU_V7 > > x86 and arm64 are: > select ARCH_HAS_STRICT_KERNEL_RWX > select ARCH_HAS_STRICT_MODULE_RWX Looks to me like it will do the job. In passing, I noticed that, on ARM: 3 .rodata 002212b4 c0703000 c0703000 00703000 2**6 CONTENTS, ALLOC, LOAD, DATA a lack of READONLY there - which suggests something in this lot isn't actually read-only data: .rodata : AT(ADDR(.rodata) - LOAD_OFFSET) { \ VMLINUX_SYMBOL(__start_rodata) = .; \ *(.rodata) *(.rodata.*) \ RO_AFTER_INIT_DATA /* Read only after init */ \ KEEP(*(__vermagic)) /* Kernel version magic */ \ . = ALIGN(8); \ VMLINUX_SYMBOL(__start___tracepoints_ptrs) = .; \ KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer array */ \ VMLINUX_SYMBOL(__stop___tracepoints_ptrs) = .; \ *(__tracepoints_strings)/* Tracepoints: strings */ \ } \ I suspect it's the RO_AFTER_INIT_DATA section - maybe this should be placed into its own separate output section? I don't think it's doing any harm, it's just odd that the .rodata section is marked as a normal data section. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common 2017-02-03 22:28 ` Russell King - ARM Linux @ 2017-02-03 23:07 ` Kees Cook 0 siblings, 0 replies; 15+ messages in thread From: Kees Cook @ 2017-02-03 23:07 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Laura Abbott, Jason Wessel, Jonathan Corbet, Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller, Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86@kernel.org, Rob Herring, Rafael J. Wysocki, Len Brown, Pavel Machek, Mark Rutland, Jessica Yu, linux-doc@vger.kernel.org On Fri, Feb 3, 2017 at 2:28 PM, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Fri, Feb 03, 2017 at 01:08:40PM -0800, Kees Cook wrote: >> On Fri, Feb 3, 2017 at 12:29 PM, Russell King - ARM Linux >> <linux@armlinux.org.uk> wrote: >> > On Fri, Feb 03, 2017 at 11:45:56AM -0800, Kees Cook wrote: >> >> On Fri, Feb 3, 2017 at 9:52 AM, Laura Abbott <labbott@redhat.com> wrote: >> >> > diff --git a/arch/Kconfig b/arch/Kconfig >> >> > index 99839c2..22ee01e 100644 >> >> > --- a/arch/Kconfig >> >> > +++ b/arch/Kconfig >> >> > @@ -781,4 +781,32 @@ config VMAP_STACK >> >> > the stack to map directly to the KASAN shadow map using a formula >> >> > that is incorrect if the stack is in vmalloc space. >> >> > >> >> > +config ARCH_NO_STRICT_RWX_DEFAULTS >> >> > + def_bool n >> >> > + >> >> > +config ARCH_HAS_STRICT_KERNEL_RWX >> >> > + def_bool n >> >> > + >> >> > +config DEBUG_RODATA >> >> > + def_bool y if !ARCH_NO_STRICT_RWX_DEFAULTS >> >> > + prompt "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS >> >> >> >> Ah! Yes, perfect. I totally forgot about using conditional "prompt" >> >> lines. Nice! >> > >> > It's no different from the more usual: >> > >> > bool "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS >> > default y if !ARCH_NO_STRICT_RWX_DEFAULTS >> > depends on ARCH_HAS_STRICT_KERNEL_RWX >> > >> > But... I really don't like this - way too many negations and negatives >> > which make it difficult to figure out what's going on here. >> > >> > The situation we have today is: >> > >> > -config DEBUG_RODATA >> > - bool "Make kernel text and rodata read-only" >> > - depends on MMU && !XIP_KERNEL >> > - default y if CPU_V7 >> > >> > which is "allow the user to select DEBUG_RODATA if building a MMU non-XIP >> > kernel", suggesting that the user turns it on for ARMv7 CPUs. >> > >> > That changes with this and the above: >> > >> > + select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL >> > + select ARCH_HAS_STRICT_MODULE_RWX if MMU >> > + select ARCH_NO_STRICT_RWX_DEFAULTS if !CPU_V7 >> > >> > This means that ARCH_HAS_STRICT_KERNEL_RWX is set for a MMU non-XIP >> > kernel, which carries the same pre-condition for DEBUG_RODATA - no >> > problem there. >> > >> > However, ARCH_NO_STRICT_RWX_DEFAULTS is set for non-ARMv7 CPUs, which >> > means the "Make kernel text and rodata read-only" prompt _is_ provided >> > for those. However, for all ARMv7 systems, we go from "suggesting that >> > the user enables the option" to "you don't have a choice, you get this >> > whether you want it or not." >> > >> > I'd prefer to keep it off for my development systems, where I don't >> > care about kernel security. If we don't wish to do that as a general >> > rule, can we make it dependent on EMBEDDED? >> > >> > Given that on ARM it can add up to 4MB to the kernel image - there >> > _will_ be about 1MB before the .text section, the padding on between >> > __modver and __ex_table which for me is around 626k, the padding >> > between .notes and the init sections start with .vectors (the space >> > between __ex_table and end of .notes is only 4124, which gets padded >> > up to 1MB) and lastly the padding between the .init section and the >> > data section (for me around 593k). This all adds up to an increase >> > in kernel image size of 3.2MB on 14.2MB - an increase of 22%. >> > >> > So no, I'm really not happy with that. >> >> Ah yeah, good point. We have three cases: unsupported, mandatory, >> optional, but we have the case of setting the default for the optional >> case. Maybe something like this? >> >> config STRICT_KERNEL_RWX >> bool "Make kernel text and rodata read-only" if ARCH_OPTIONAL_KERNEL_RWX >> depends on ARCH_HAS_STRICT_KERNEL_RWX >> default ARCH_OPTIONAL_KERNEL_RWX_DEFAULT >> >> unsupported: >> !ARCH_HAS_STRICT_KERNEL_RWX >> >> mandatory: >> ARCH_HAS_STRICT_KERNEL_RWX >> !ARCH_OPTIONAL_KERNEL_RWX >> >> optional: >> ARCH_HAS_STRICT_KERNEL_RWX >> ARCH_OPTIONAL_KERNEL_RWX >> with default controlled by ARCH_OPTIONAL_KERNEL_RWX_DEFAULT >> >> Then arm is: >> select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL >> select ARCH_HAS_STRICT_MODULE_RWX if MMU >> select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX >> select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT if CPU_V7 >> >> x86 and arm64 are: >> select ARCH_HAS_STRICT_KERNEL_RWX >> select ARCH_HAS_STRICT_MODULE_RWX > > Looks to me like it will do the job. > > In passing, I noticed that, on ARM: > > 3 .rodata 002212b4 c0703000 c0703000 00703000 2**6 > CONTENTS, ALLOC, LOAD, DATA > > a lack of READONLY there - which suggests something in this lot isn't > actually read-only data: > > .rodata : AT(ADDR(.rodata) - LOAD_OFFSET) { \ > VMLINUX_SYMBOL(__start_rodata) = .; \ > *(.rodata) *(.rodata.*) \ > RO_AFTER_INIT_DATA /* Read only after init */ \ > KEEP(*(__vermagic)) /* Kernel version magic */ \ > . = ALIGN(8); \ > VMLINUX_SYMBOL(__start___tracepoints_ptrs) = .; \ > KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer array */ \ > VMLINUX_SYMBOL(__stop___tracepoints_ptrs) = .; \ > *(__tracepoints_strings)/* Tracepoints: strings */ \ > } \ > > I suspect it's the RO_AFTER_INIT_DATA section - maybe this should be > placed into its own separate output section? I don't think it's doing > any harm, it's just odd that the .rodata section is marked as a normal > data section. Yeah, I think the linker drops the RO flag when it see the RO_AFTER_INIT_DATA flags. Since the kernel does the permissions settings, this has been fine. If we can force the linker flags on it, I'm all for it, but every time I've tried, it has defeated me. :) -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common 2017-02-03 21:08 ` Kees Cook 2017-02-03 22:28 ` Russell King - ARM Linux @ 2017-02-06 18:47 ` Laura Abbott 2017-02-07 7:36 ` Pavel Machek 1 sibling, 1 reply; 15+ messages in thread From: Laura Abbott @ 2017-02-06 18:47 UTC (permalink / raw) To: Kees Cook, Russell King - ARM Linux Cc: Jason Wessel, Jonathan Corbet, Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller, Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86@kernel.org, Rob Herring, Rafael J. Wysocki, Len Brown, Pavel Machek, Mark Rutland, Jessica Yu, linux-doc@vger.kernel.org, LKML On 02/03/2017 01:08 PM, Kees Cook wrote: > On Fri, Feb 3, 2017 at 12:29 PM, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: >> On Fri, Feb 03, 2017 at 11:45:56AM -0800, Kees Cook wrote: >>> On Fri, Feb 3, 2017 at 9:52 AM, Laura Abbott <labbott@redhat.com> wrote: >>>> diff --git a/arch/Kconfig b/arch/Kconfig >>>> index 99839c2..22ee01e 100644 >>>> --- a/arch/Kconfig >>>> +++ b/arch/Kconfig >>>> @@ -781,4 +781,32 @@ config VMAP_STACK >>>> the stack to map directly to the KASAN shadow map using a formula >>>> that is incorrect if the stack is in vmalloc space. >>>> >>>> +config ARCH_NO_STRICT_RWX_DEFAULTS >>>> + def_bool n >>>> + >>>> +config ARCH_HAS_STRICT_KERNEL_RWX >>>> + def_bool n >>>> + >>>> +config DEBUG_RODATA >>>> + def_bool y if !ARCH_NO_STRICT_RWX_DEFAULTS >>>> + prompt "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS >>> >>> Ah! Yes, perfect. I totally forgot about using conditional "prompt" >>> lines. Nice! >> >> It's no different from the more usual: >> >> bool "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS >> default y if !ARCH_NO_STRICT_RWX_DEFAULTS >> depends on ARCH_HAS_STRICT_KERNEL_RWX >> >> But... I really don't like this - way too many negations and negatives >> which make it difficult to figure out what's going on here. >> >> The situation we have today is: >> >> -config DEBUG_RODATA >> - bool "Make kernel text and rodata read-only" >> - depends on MMU && !XIP_KERNEL >> - default y if CPU_V7 >> >> which is "allow the user to select DEBUG_RODATA if building a MMU non-XIP >> kernel", suggesting that the user turns it on for ARMv7 CPUs. >> >> That changes with this and the above: >> >> + select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL >> + select ARCH_HAS_STRICT_MODULE_RWX if MMU >> + select ARCH_NO_STRICT_RWX_DEFAULTS if !CPU_V7 >> >> This means that ARCH_HAS_STRICT_KERNEL_RWX is set for a MMU non-XIP >> kernel, which carries the same pre-condition for DEBUG_RODATA - no >> problem there. >> >> However, ARCH_NO_STRICT_RWX_DEFAULTS is set for non-ARMv7 CPUs, which >> means the "Make kernel text and rodata read-only" prompt _is_ provided >> for those. However, for all ARMv7 systems, we go from "suggesting that >> the user enables the option" to "you don't have a choice, you get this >> whether you want it or not." >> >> I'd prefer to keep it off for my development systems, where I don't >> care about kernel security. If we don't wish to do that as a general >> rule, can we make it dependent on EMBEDDED? >> >> Given that on ARM it can add up to 4MB to the kernel image - there >> _will_ be about 1MB before the .text section, the padding on between >> __modver and __ex_table which for me is around 626k, the padding >> between .notes and the init sections start with .vectors (the space >> between __ex_table and end of .notes is only 4124, which gets padded >> up to 1MB) and lastly the padding between the .init section and the >> data section (for me around 593k). This all adds up to an increase >> in kernel image size of 3.2MB on 14.2MB - an increase of 22%. >> >> So no, I'm really not happy with that. > > Ah yeah, good point. We have three cases: unsupported, mandatory, > optional, but we have the case of setting the default for the optional > case. Maybe something like this? > > config STRICT_KERNEL_RWX > bool "Make kernel text and rodata read-only" if ARCH_OPTIONAL_KERNEL_RWX > depends on ARCH_HAS_STRICT_KERNEL_RWX > default ARCH_OPTIONAL_KERNEL_RWX_DEFAULT > > unsupported: > !ARCH_HAS_STRICT_KERNEL_RWX > > mandatory: > ARCH_HAS_STRICT_KERNEL_RWX > !ARCH_OPTIONAL_KERNEL_RWX > > optional: > ARCH_HAS_STRICT_KERNEL_RWX > ARCH_OPTIONAL_KERNEL_RWX > with default controlled by ARCH_OPTIONAL_KERNEL_RWX_DEFAULT > > Then arm is: > select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL > select ARCH_HAS_STRICT_MODULE_RWX if MMU > select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX > select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT if CPU_V7 > > x86 and arm64 are: > select ARCH_HAS_STRICT_KERNEL_RWX > select ARCH_HAS_STRICT_MODULE_RWX > > ? > > -Kees > Yes, that looks good. I wanted it to be mandatory to avoid the mindset of "optional means we don't need it" but I see there are some cases where it's better to turn it off. I'll see if I can emphasize this properly in the help text ("Say Y here unless you love security exploits running in production") Thanks, Laura ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common 2017-02-06 18:47 ` Laura Abbott @ 2017-02-07 7:36 ` Pavel Machek 0 siblings, 0 replies; 15+ messages in thread From: Pavel Machek @ 2017-02-07 7:36 UTC (permalink / raw) To: Laura Abbott Cc: Kees Cook, Russell King - ARM Linux, Jason Wessel, Jonathan Corbet, Catalin Marinas, Will Deacon, James E.J. Bottomley, Helge Deller, Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86@kernel.org, Rob Herring, Rafael J. Wysocki, Len Brown, Mark Rutland, Jessica Yu [-- Attachment #1: Type: text/plain, Size: 5297 bytes --] On Mon 2017-02-06 10:47:45, Laura Abbott wrote: > On 02/03/2017 01:08 PM, Kees Cook wrote: > > On Fri, Feb 3, 2017 at 12:29 PM, Russell King - ARM Linux > > <linux@armlinux.org.uk> wrote: > >> On Fri, Feb 03, 2017 at 11:45:56AM -0800, Kees Cook wrote: > >>> On Fri, Feb 3, 2017 at 9:52 AM, Laura Abbott <labbott@redhat.com> wrote: > >>>> diff --git a/arch/Kconfig b/arch/Kconfig > >>>> index 99839c2..22ee01e 100644 > >>>> --- a/arch/Kconfig > >>>> +++ b/arch/Kconfig > >>>> @@ -781,4 +781,32 @@ config VMAP_STACK > >>>> the stack to map directly to the KASAN shadow map using a formula > >>>> that is incorrect if the stack is in vmalloc space. > >>>> > >>>> +config ARCH_NO_STRICT_RWX_DEFAULTS > >>>> + def_bool n > >>>> + > >>>> +config ARCH_HAS_STRICT_KERNEL_RWX > >>>> + def_bool n > >>>> + > >>>> +config DEBUG_RODATA > >>>> + def_bool y if !ARCH_NO_STRICT_RWX_DEFAULTS > >>>> + prompt "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS > >>> > >>> Ah! Yes, perfect. I totally forgot about using conditional "prompt" > >>> lines. Nice! > >> > >> It's no different from the more usual: > >> > >> bool "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS > >> default y if !ARCH_NO_STRICT_RWX_DEFAULTS > >> depends on ARCH_HAS_STRICT_KERNEL_RWX > >> > >> But... I really don't like this - way too many negations and negatives > >> which make it difficult to figure out what's going on here. > >> > >> The situation we have today is: > >> > >> -config DEBUG_RODATA > >> - bool "Make kernel text and rodata read-only" > >> - depends on MMU && !XIP_KERNEL > >> - default y if CPU_V7 > >> > >> which is "allow the user to select DEBUG_RODATA if building a MMU non-XIP > >> kernel", suggesting that the user turns it on for ARMv7 CPUs. > >> > >> That changes with this and the above: > >> > >> + select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL > >> + select ARCH_HAS_STRICT_MODULE_RWX if MMU > >> + select ARCH_NO_STRICT_RWX_DEFAULTS if !CPU_V7 > >> > >> This means that ARCH_HAS_STRICT_KERNEL_RWX is set for a MMU non-XIP > >> kernel, which carries the same pre-condition for DEBUG_RODATA - no > >> problem there. > >> > >> However, ARCH_NO_STRICT_RWX_DEFAULTS is set for non-ARMv7 CPUs, which > >> means the "Make kernel text and rodata read-only" prompt _is_ provided > >> for those. However, for all ARMv7 systems, we go from "suggesting that > >> the user enables the option" to "you don't have a choice, you get this > >> whether you want it or not." > >> > >> I'd prefer to keep it off for my development systems, where I don't > >> care about kernel security. If we don't wish to do that as a general > >> rule, can we make it dependent on EMBEDDED? > >> > >> Given that on ARM it can add up to 4MB to the kernel image - there > >> _will_ be about 1MB before the .text section, the padding on between > >> __modver and __ex_table which for me is around 626k, the padding > >> between .notes and the init sections start with .vectors (the space > >> between __ex_table and end of .notes is only 4124, which gets padded > >> up to 1MB) and lastly the padding between the .init section and the > >> data section (for me around 593k). This all adds up to an increase > >> in kernel image size of 3.2MB on 14.2MB - an increase of 22%. > >> > >> So no, I'm really not happy with that. > > > > Ah yeah, good point. We have three cases: unsupported, mandatory, > > optional, but we have the case of setting the default for the optional > > case. Maybe something like this? > > > > config STRICT_KERNEL_RWX > > bool "Make kernel text and rodata read-only" if ARCH_OPTIONAL_KERNEL_RWX > > depends on ARCH_HAS_STRICT_KERNEL_RWX > > default ARCH_OPTIONAL_KERNEL_RWX_DEFAULT > > > > unsupported: > > !ARCH_HAS_STRICT_KERNEL_RWX > > > > mandatory: > > ARCH_HAS_STRICT_KERNEL_RWX > > !ARCH_OPTIONAL_KERNEL_RWX > > > > optional: > > ARCH_HAS_STRICT_KERNEL_RWX > > ARCH_OPTIONAL_KERNEL_RWX > > with default controlled by ARCH_OPTIONAL_KERNEL_RWX_DEFAULT > > > > Then arm is: > > select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL > > select ARCH_HAS_STRICT_MODULE_RWX if MMU > > select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX > > select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT if CPU_V7 > > > > x86 and arm64 are: > > select ARCH_HAS_STRICT_KERNEL_RWX > > select ARCH_HAS_STRICT_MODULE_RWX > > > > ? > > > > -Kees > > > > Yes, that looks good. I wanted it to be mandatory to avoid the > mindset of "optional means we don't need it" but I see there > are some cases where it's better to turn it off. I'll see if > I can emphasize this properly in the help text ("Say Y here > unless you love security exploits running in production") What about fixing the memory wastage, instead? If you want something almost-always-on, it should not waste megabytes of memory. And BTW it is help text, not advertising for your favourite feature. 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] 15+ messages in thread
* [PATCHv2 2/2] arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX 2017-02-03 17:52 [PATCHv2 0/2] Hardening configs refactor/rename Laura Abbott 2017-02-03 17:52 ` [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common Laura Abbott @ 2017-02-03 17:52 ` Laura Abbott 2017-02-03 18:26 ` Mark Rutland 2017-02-03 20:03 ` Kees Cook 1 sibling, 2 replies; 15+ messages in thread From: Laura Abbott @ 2017-02-03 17:52 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-ker Both of these options are poorly named. The features they provide are necessary for system security and should not be considered debug only. Change the name to something that accurately describes what these options do. Signed-off-by: Laura Abbott <labbott@redhat.com> --- v2: This patch is now doing the renaming of CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX --- Documentation/DocBook/kgdb.tmpl | 8 ++++---- Documentation/security/self-protection.txt | 4 ++-- arch/Kconfig | 4 ++-- arch/arm/configs/aspeed_g4_defconfig | 3 +-- arch/arm/configs/aspeed_g5_defconfig | 3 +-- arch/arm/include/asm/cacheflush.h | 2 +- arch/arm/kernel/patch.c | 4 ++-- arch/arm/kernel/vmlinux.lds.S | 8 ++++---- arch/arm/mm/Kconfig | 2 +- arch/arm/mm/init.c | 4 ++-- arch/arm64/Kconfig.debug | 2 +- arch/arm64/kernel/insn.c | 2 +- arch/parisc/configs/712_defconfig | 1 - arch/parisc/configs/c3000_defconfig | 1 - arch/parisc/mm/init.c | 2 +- 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 ++-- 24 files changed, 39 insertions(+), 43 deletions(-) diff --git a/Documentation/DocBook/kgdb.tmpl b/Documentation/DocBook/kgdb.tmpl index f3abca7..856ac20 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_STRICT_KERNEL_RWX, 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_STRICT_KERNEL_RWX 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_STRICT_KERNEL_RWX 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_STRICT_KERNEL_RWX 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..dd2a3b1 100644 --- a/Documentation/security/self-protection.txt +++ b/Documentation/security/self-protection.txt @@ -51,8 +51,8 @@ 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 -CONFIG_DEBUG_SET_MODULE_RONX, which seek to make sure that code is not +In support of this are CONFIG_STRICT_KERNEL_RWX and +CONFIG_STRICT_MODULE_RWX, 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/Kconfig b/arch/Kconfig index 22ee01e..406f6cd 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -787,7 +787,7 @@ config ARCH_NO_STRICT_RWX_DEFAULTS config ARCH_HAS_STRICT_KERNEL_RWX def_bool n -config DEBUG_RODATA +config STRICT_KERNEL_RWX def_bool y if !ARCH_NO_STRICT_RWX_DEFAULTS prompt "Make kernel text and rodata read-only" if ARCH_NO_STRICT_RWX_DEFAULTS depends on ARCH_HAS_STRICT_KERNEL_RWX @@ -800,7 +800,7 @@ config DEBUG_RODATA config ARCH_HAS_STRICT_MODULE_RWX def_bool n -config DEBUG_SET_MODULE_RONX +config STRICT_MODULE_RWX def_bool y if !ARCH_NO_STRICT_RWX_DEFAULTS prompt "Set loadable kenrel module data as NX and text as RO" if ARCH_NO_STRICT_RWX_DEFAULTS depends on ARCH_HAS_STRICT_MODULE_RWX && MODULES diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig index ca39c04..beea2cc 100644 --- a/arch/arm/configs/aspeed_g4_defconfig +++ b/arch/arm/configs/aspeed_g4_defconfig @@ -25,7 +25,6 @@ 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_AEABI=y CONFIG_UACCESS_WITH_MEMCPY=y CONFIG_SECCOMP=y @@ -79,7 +78,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_STRICT_MODULE_RWX=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 4f366b0..0e245e5 100644 --- a/arch/arm/configs/aspeed_g5_defconfig +++ b/arch/arm/configs/aspeed_g5_defconfig @@ -26,7 +26,6 @@ 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_AEABI=y CONFIG_UACCESS_WITH_MEMCPY=y CONFIG_SECCOMP=y @@ -81,7 +80,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_STRICT_MODULE_RWX=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/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h index bdd283b..02454fa 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_STRICT_KERNEL_RWX 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..020560b 100644 --- a/arch/arm/kernel/patch.c +++ b/arch/arm/kernel/patch.c @@ -24,9 +24,9 @@ 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_STRICT_MODULE_RWX)) page = vmalloc_to_page(addr); - else if (!module && IS_ENABLED(CONFIG_DEBUG_RODATA)) + else if (!module && IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) 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..ce18007 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_STRICT_KERNEL_RWX . = ALIGN(1<<SECTION_SHIFT); #endif @@ -158,7 +158,7 @@ SECTIONS NOTES -#ifdef CONFIG_DEBUG_RODATA +#ifdef CONFIG_STRICT_KERNEL_RWX . = 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_STRICT_KERNEL_RWX . = ALIGN(1<<SECTION_SHIFT); #else . = ALIGN(THREAD_SIZE); @@ -325,7 +325,7 @@ SECTIONS STABS_DEBUG } -#ifdef CONFIG_DEBUG_RODATA +#ifdef CONFIG_STRICT_KERNEL_RWX /* * 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 419a035..35e3a56 100644 --- a/arch/arm/mm/Kconfig +++ b/arch/arm/mm/Kconfig @@ -1053,7 +1053,7 @@ config ARCH_SUPPORTS_BIG_ENDIAN config DEBUG_ALIGN_RODATA bool "Make rodata strictly non-executable" - depends on DEBUG_RODATA + depends on STRICT_KERNEL_RWX 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..4be0bee 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_STRICT_KERNEL_RWX 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_STRICT_KERNEL_RWX */ void free_tcmmem(void) { diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug index 939815e..560a8d8 100644 --- a/arch/arm64/Kconfig.debug +++ b/arch/arm64/Kconfig.debug @@ -72,7 +72,7 @@ config DEBUG_WX If in doubt, say "Y". config DEBUG_ALIGN_RODATA - depends on DEBUG_RODATA + depends on STRICT_KERNEL_RWX 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/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index 94b62c1..67f9cb9 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_STRICT_MODULE_RWX)) page = vmalloc_to_page(addr); else if (!module) page = pfn_to_page(PHYS_PFN(__pa(addr))); diff --git a/arch/parisc/configs/712_defconfig b/arch/parisc/configs/712_defconfig index db8f56b..143d026 100644 --- a/arch/parisc/configs/712_defconfig +++ b/arch/parisc/configs/712_defconfig @@ -182,7 +182,6 @@ 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_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..8e8f0e3 100644 --- a/arch/parisc/configs/c3000_defconfig +++ b/arch/parisc/configs/c3000_defconfig @@ -166,7 +166,6 @@ 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_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..a055e5b 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_STRICT_KERNEL_RWX void mark_rodata_ro(void) { /* rodata memory was already mapped with KERNEL_RO access rights by diff --git a/include/linux/filter.h b/include/linux/filter.h index e4eb254..c2d2827 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_STRICT_MODULE_RWX 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_STRICT_MODULE_RWX */ 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 885c3e6..79af096 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_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX) extern bool rodata_enabled; #endif -#ifdef CONFIG_DEBUG_RODATA +#ifdef CONFIG_STRICT_KERNEL_RWX void mark_rodata_ro(void); #endif diff --git a/include/linux/module.h b/include/linux/module.h index 7c84273..d5afd14 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_STRICT_MODULE_RWX 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 b0c9d6f..0b7bae2 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_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX) 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_STRICT_KERNEL_RWX static void mark_readonly(void) { if (rodata_enabled) diff --git a/kernel/configs/android-recommended.config b/kernel/configs/android-recommended.config index 297756b..99127ed 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_STRICT_KERNEL_RWX=y CONFIG_DM_CRYPT=y CONFIG_DM_UEVENT=y CONFIG_DM_VERITY=y diff --git a/kernel/module.c b/kernel/module.c index 38d4270..2643a14 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_STRICT_MODULE_RWX=y */ -#ifdef CONFIG_DEBUG_SET_MODULE_RONX +#ifdef CONFIG_STRICT_MODULE_RWX # 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_STRICT_MODULE_RWX /* * LKM RO/NX protection: protect module's text/ro-data * from modification and any data from execution. diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index b26dbc4..86385af 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_STRICT_KERNEL_RWX) && !strncmp(str, "protect_image", 13)) { enable_restore_image_protection(); } diff --git a/kernel/power/power.h b/kernel/power/power.h index 1dfa0da..7fdc40d 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_STRICT_KERNEL_RWX /* 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_STRICT_KERNEL_RWX */ #else /* !CONFIG_HIBERNATION */ diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index 2d8e2b2..905d5bb 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -38,7 +38,7 @@ #include "power.h" -#ifdef CONFIG_DEBUG_RODATA +#ifdef CONFIG_STRICT_KERNEL_RWX 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_STRICT_KERNEL_RWX */ static int swsusp_page_is_free(struct page *); static void swsusp_set_page_forbidden(struct page *); -- 2.7.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCHv2 2/2] arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX 2017-02-03 17:52 ` [PATCHv2 2/2] arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX Laura Abbott @ 2017-02-03 18:26 ` Mark Rutland 2017-02-03 20:03 ` Kees Cook 1 sibling, 0 replies; 15+ messages in thread From: Mark Rutland @ 2017-02-03 18:26 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 On Fri, Feb 03, 2017 at 09:52:22AM -0800, Laura Abbott wrote: > > Both of these options are poorly named. The features they provide are > necessary for system security and should not be considered debug only. > Change the name to something that accurately describes what these > options do. > > Signed-off-by: Laura Abbott <labbott@redhat.com> As with patch 1, this looks good to me. FWIW: Acked-by: Mark Rutland <mark.rutland@arm.com> > diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug > index 939815e..560a8d8 100644 > --- a/arch/arm64/Kconfig.debug > +++ b/arch/arm64/Kconfig.debug > @@ -72,7 +72,7 @@ config DEBUG_WX > If in doubt, say "Y". > > config DEBUG_ALIGN_RODATA > - depends on DEBUG_RODATA > + depends on STRICT_KERNEL_RWX > 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/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c > index 94b62c1..67f9cb9 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_STRICT_MODULE_RWX)) > page = vmalloc_to_page(addr); > else if (!module) > page = pfn_to_page(PHYS_PFN(__pa(addr))); Since CONFIG_STRICT_MODULE_RWX is mandatory for arm64, we should be able to simplify this, but that can wait for a later patch. Thanks, Mark. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2 2/2] arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX 2017-02-03 17:52 ` [PATCHv2 2/2] arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX Laura Abbott 2017-02-03 18:26 ` Mark Rutland @ 2017-02-03 20:03 ` Kees Cook 2017-02-06 18:49 ` Laura Abbott 1 sibling, 1 reply; 15+ messages in thread From: Kees Cook @ 2017-02-03 20:03 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@kernel.org, Rob Herring, Rafael J. Wysocki, Len Brown, Pavel Machek, Mark Rutland, Jessica Yu, linux-doc@vger.kernel.org On Fri, Feb 3, 2017 at 9:52 AM, Laura Abbott <labbott@redhat.com> wrote: > > Both of these options are poorly named. The features they provide are > necessary for system security and should not be considered debug only. > Change the name to something that accurately describes what these > options do. It may help to explicitly call out the name change from/to in the commit message. > > Signed-off-by: Laura Abbott <labbott@redhat.com> > --- > [...] > diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig > index ca39c04..beea2cc 100644 > --- a/arch/arm/configs/aspeed_g4_defconfig > +++ b/arch/arm/configs/aspeed_g4_defconfig > @@ -25,7 +25,6 @@ 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_AEABI=y > CONFIG_UACCESS_WITH_MEMCPY=y > CONFIG_SECCOMP=y Are these defconfig cases correct (dropping DEBUG_RODATA without adding STRICT_KERNEL_RWX)? Who should carry this series, btw? -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2 2/2] arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX 2017-02-03 20:03 ` Kees Cook @ 2017-02-06 18:49 ` Laura Abbott 2017-02-06 20:13 ` Kees Cook 0 siblings, 1 reply; 15+ messages in thread From: Laura Abbott @ 2017-02-06 18:49 UTC (permalink / raw) To: Kees Cook 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@kernel.org, Rob Herring, Rafael J. Wysocki, Len Brown, Pavel Machek, Mark Rutland, Jessica Yu, linux-doc@vger.kernel.org On 02/03/2017 12:03 PM, Kees Cook wrote: > On Fri, Feb 3, 2017 at 9:52 AM, Laura Abbott <labbott@redhat.com> wrote: >> >> Both of these options are poorly named. The features they provide are >> necessary for system security and should not be considered debug only. >> Change the name to something that accurately describes what these >> options do. > > It may help to explicitly call out the name change from/to in the > commit message. > >> >> Signed-off-by: Laura Abbott <labbott@redhat.com> >> --- >> [...] >> diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig >> index ca39c04..beea2cc 100644 >> --- a/arch/arm/configs/aspeed_g4_defconfig >> +++ b/arch/arm/configs/aspeed_g4_defconfig >> @@ -25,7 +25,6 @@ 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_AEABI=y >> CONFIG_UACCESS_WITH_MEMCPY=y >> CONFIG_SECCOMP=y > > Are these defconfig cases correct (dropping DEBUG_RODATA without > adding STRICT_KERNEL_RWX)? > Yes, I think these need to be updated to the new config option since these are not CPUv7 > Who should carry this series, btw? > An excellent question :) Would you be willing to carry it with Acks? > -Kees > Thanks, Laura ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2 2/2] arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX 2017-02-06 18:49 ` Laura Abbott @ 2017-02-06 20:13 ` Kees Cook 0 siblings, 0 replies; 15+ messages in thread From: Kees Cook @ 2017-02-06 20:13 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@kernel.org, Rob Herring, Rafael J. Wysocki, Len Brown, Pavel Machek, Mark Rutland, Jessica Yu, linux-doc@vger.kernel.org On Mon, Feb 6, 2017 at 10:49 AM, Laura Abbott <labbott@redhat.com> wrote: > On 02/03/2017 12:03 PM, Kees Cook wrote: >> On Fri, Feb 3, 2017 at 9:52 AM, Laura Abbott <labbott@redhat.com> wrote: >>> >>> Both of these options are poorly named. The features they provide are >>> necessary for system security and should not be considered debug only. >>> Change the name to something that accurately describes what these >>> options do. >> >> It may help to explicitly call out the name change from/to in the >> commit message. >> >>> >>> Signed-off-by: Laura Abbott <labbott@redhat.com> >>> --- >>> [...] >>> diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig >>> index ca39c04..beea2cc 100644 >>> --- a/arch/arm/configs/aspeed_g4_defconfig >>> +++ b/arch/arm/configs/aspeed_g4_defconfig >>> @@ -25,7 +25,6 @@ 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_AEABI=y >>> CONFIG_UACCESS_WITH_MEMCPY=y >>> CONFIG_SECCOMP=y >> >> Are these defconfig cases correct (dropping DEBUG_RODATA without >> adding STRICT_KERNEL_RWX)? >> > > Yes, I think these need to be updated to the new config option since > these are not CPUv7 > > >> Who should carry this series, btw? >> > > An excellent question :) > > Would you be willing to carry it with Acks? Yeah, I can push this via the KSPP tree: it's cross-architecture, so it seems like this should go either through my tree or through akpm's tree. Are the arch maintainers okay with that? -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-02-07 7:36 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-02-03 17:52 [PATCHv2 0/2] Hardening configs refactor/rename Laura Abbott 2017-02-03 17:52 ` [PATCHv2 1/2] arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common Laura Abbott 2017-02-03 18:16 ` Mark Rutland 2017-02-03 19:45 ` Kees Cook 2017-02-03 20:29 ` Russell King - ARM Linux 2017-02-03 21:08 ` Kees Cook 2017-02-03 22:28 ` Russell King - ARM Linux 2017-02-03 23:07 ` Kees Cook 2017-02-06 18:47 ` Laura Abbott 2017-02-07 7:36 ` Pavel Machek 2017-02-03 17:52 ` [PATCHv2 2/2] arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX Laura Abbott 2017-02-03 18:26 ` Mark Rutland 2017-02-03 20:03 ` Kees Cook 2017-02-06 18:49 ` Laura Abbott 2017-02-06 20:13 ` 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).