| Message ID | 1396346657-7166-1-git-send-email-holler@ahsoftware.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 4/1/2014 3:04 AM, Alexander Holler wrote: > CONFIG_DEBUG_SET_MODULE_RONX sounds like a nice security feature, but > things might fail late (and unexpected) if module code is set to read-only > while CONFIG_JUMP_LABEL is enabled (e.g. modprobe bridge). > > Avoid this. > > Signed-off-by: Alexander Holler <holler@ahsoftware.de> > --- > arch/arm/Kconfig.debug | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug > index 0531da8..6627b9e 100644 > --- a/arch/arm/Kconfig.debug > +++ b/arch/arm/Kconfig.debug > @@ -1197,7 +1197,7 @@ config PID_IN_CONTEXTIDR > > config DEBUG_SET_MODULE_RONX > bool "Set loadable kernel module data as NX and text as RO" > - depends on MODULES > + depends on MODULES && !JUMP_LABEL > ---help--- > This option helps catch unintended modifications to loadable > kernel module's text and read-only data. It also prevents execution > Kees Cook has something similar[1] for not-module space as well, we probably want this there as well. A shame we keep finding reasons these features will be turned off. Looks good to me otherwise. Laura [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2014-February/232644.html
On Tue, Apr 1, 2014 at 11:03 AM, Laura Abbott <lauraa@codeaurora.org> wrote: > On 4/1/2014 3:04 AM, Alexander Holler wrote: >> CONFIG_DEBUG_SET_MODULE_RONX sounds like a nice security feature, but >> things might fail late (and unexpected) if module code is set to read-only >> while CONFIG_JUMP_LABEL is enabled (e.g. modprobe bridge). Isn't this a ordering problem? I thought jump labels got set up once, and then after that, the memory could be made RO? >> >> Avoid this. >> >> Signed-off-by: Alexander Holler <holler@ahsoftware.de> >> --- >> arch/arm/Kconfig.debug | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug >> index 0531da8..6627b9e 100644 >> --- a/arch/arm/Kconfig.debug >> +++ b/arch/arm/Kconfig.debug >> @@ -1197,7 +1197,7 @@ config PID_IN_CONTEXTIDR >> >> config DEBUG_SET_MODULE_RONX >> bool "Set loadable kernel module data as NX and text as RO" >> - depends on MODULES >> + depends on MODULES && !JUMP_LABEL >> ---help--- >> This option helps catch unintended modifications to loadable >> kernel module's text and read-only data. It also prevents execution >> > > Kees Cook has something similar[1] for not-module space as well, we probably want > this there as well. A shame we keep finding reasons these features will be turned > off. Looks good to me otherwise. > > Laura > > [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2014-February/232644.html I've solidly stumbled over ftrace with my series now. I can't figure out why it doesn't work, though. I wrote code to flip the PMD flags back to writable (via set_kernel_text_rw), but it seems like the system is ignoring the changes, even though I call flush_pmd_entry. struct section_perm section_perms[] = { /* Make kernel code and rodata RX (set RO). */ [ARM_TEXT_SECTION] = { .start = (unsigned long)_stext, .end = (unsigned long)__init_begin, #ifdef CONFIG_ARM_LPAE .mask = ~PMD_SECT_RDONLY, .prot = PMD_SECT_RDONLY, #else .mask = ~(PMD_SECT_APX | PMD_SECT_AP_WRITE), .prot = PMD_SECT_APX | PMD_SECT_AP_WRITE, .clear = PMD_SECT_AP_WRITE, #endif }, ... static inline void section_update(unsigned long addr, pmdval_t mask, pmdval_t prot) { pmd_t *pmd = pmd_off_k(addr); #ifdef CONFIG_ARM_LPAE pmd[0] = __pmd((pmd_val(pmd[0]) & mask) | prot); #else if (addr & SECTION_SIZE) pmd[1] = __pmd((pmd_val(pmd[1]) & mask) | prot); else pmd[0] = __pmd((pmd_val(pmd[0]) & mask) | prot); #endif flush_pmd_entry(pmd); } ... void set_kernel_text_rw(void) { unsigned long addr; if (!arch_can_set_perms()) return; for (addr = section_perms[ARM_TEXT_SECTION].start; addr < section_perms[ARM_TEXT_SECTION].end; addr += SECTION_SIZE) section_update(addr, section_perms[ARM_TEXT_SECTION].mask, section_perms[ARM_TEXT_SECTION].clear); } Is there something "sticky" about PMD sections that I'm not aware of? Even after calling set_kernel_text_rw(), any writes to kernel memory fault. :( -Kees
Am 01.04.2014 20:36, schrieb Kees Cook: > On Tue, Apr 1, 2014 at 11:03 AM, Laura Abbott <lauraa@codeaurora.org> wrote: >> On 4/1/2014 3:04 AM, Alexander Holler wrote: >>> CONFIG_DEBUG_SET_MODULE_RONX sounds like a nice security feature, but >>> things might fail late (and unexpected) if module code is set to read-only >>> while CONFIG_JUMP_LABEL is enabled (e.g. modprobe bridge). > > Isn't this a ordering problem? I thought jump labels got set up once, > and then after that, the memory could be made RO? I basically just run into that and looked up what happened. But the problem appears e.g. in netfiler/core.c function nf_register_hook() which calls static_key_slow_inc(). So you would have to make sure nf_register_hook() will be called before the code is set ro. Something that doesn't look easy to do. I would have to look up when that might be called, but I assume there are many ways to register and unregister hooks in netfilter and some of them might happen outside any init, probe or whatever one might set the code read-only afterwards. You would have to set the code rw too, before nf_unregister_hook() happens. Maybe it's possible to mark some modules to not become ro at all, I don't know. And doing so would make them a prefered target for exploits. So I'm not sure if it would make sense. Regards, Alexander Holler
2014-04-01 20:36 GMT+02:00 Kees Cook <keescook@chromium.org>: > On Tue, Apr 1, 2014 at 11:03 AM, Laura Abbott <lauraa@codeaurora.org> wrote: >> On 4/1/2014 3:04 AM, Alexander Holler wrote: >>> CONFIG_DEBUG_SET_MODULE_RONX sounds like a nice security feature, but >>> things might fail late (and unexpected) if module code is set to read-only >>> while CONFIG_JUMP_LABEL is enabled (e.g. modprobe bridge). > > Isn't this a ordering problem? I thought jump labels got set up once, > and then after that, the memory could be made RO? The code gets patched every time the jump label is enabled/disabled, i.e. every time the static_key is turned on/off based on static_key_slow_{inc/dec}()
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug index 0531da8..6627b9e 100644 --- a/arch/arm/Kconfig.debug +++ b/arch/arm/Kconfig.debug @@ -1197,7 +1197,7 @@ config PID_IN_CONTEXTIDR config DEBUG_SET_MODULE_RONX bool "Set loadable kernel module data as NX and text as RO" - depends on MODULES + depends on MODULES && !JUMP_LABEL ---help--- This option helps catch unintended modifications to loadable kernel module's text and read-only data. It also prevents execution
CONFIG_DEBUG_SET_MODULE_RONX sounds like a nice security feature, but things might fail late (and unexpected) if module code is set to read-only while CONFIG_JUMP_LABEL is enabled (e.g. modprobe bridge). Avoid this. Signed-off-by: Alexander Holler <holler@ahsoftware.de> --- arch/arm/Kconfig.debug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)