linux-kernel - Re: [PATCH] arm: don't allow CONFIG_DEBUG_SET_MODULE_RONX if CONFIG_JUMP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGXu5jKMF9n9svQBhyQFvCoUe86QhLycDTt-fWyiRfySbn=Fug@mail.gmail.com>
Date:	Tue, 1 Apr 2014 11:36:48 -0700
From:	Kees Cook <keescook@...omium.org>
To:	Laura Abbott <lauraa@...eaurora.org>
Cc:	Alexander Holler <holler@...oftware.de>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	LKML <linux-kernel@...r.kernel.org>,
	Russell King <linux@....linux.org.uk>
Subject: Re: [PATCH] arm: don't allow CONFIG_DEBUG_SET_MODULE_RONX if
 CONFIG_JUMP_LABEL is enabled

On Tue, Apr 1, 2014 at 11:03 AM, Laura Abbott <lauraa@...eaurora.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@...oftware.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

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/