Messages in this thread | | | Date | Mon, 5 Oct 2015 12:36:23 -0700 | Subject | Re: [PATCH v3] x86/mm: warn on W+x mappings | From | Kees Cook <> |
| |
On Mon, Oct 5, 2015 at 9:55 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > Warn on any residual W+x mappings after setting NX > if DEBUG_WX is enabled. Introduce a separate > X86_PTDUMP_CORE config that enables the code for > dumping the page tables without enabling the debugfs > interface, so that DEBUG_WX can be enabled without > exposing the debugfs interface. Switch EFI_PGT_DUMP > to using X86_PTDUMP_CORE so that it also does not require > enabling the debugfs interface. > > On success: > x86/mm: Checked W+X mappings: passed, no W+X pages found. > > On failure: > ------------[ cut here ]------------ > WARNING: CPU: 1 PID: 1 at arch/x86/mm/dump_pagetables.c:226 note_page+0x610/0x7b0() > x86/mm: Found insecure W+X mapping at address ffffffff81755000/__stop___ex_table+0xfa8/0xabfa8 > Modules linked in: > CPU: 1 PID: 1 Comm: swapper/0 Tainted: G W 4.3.0-rc3+ #19 > 0000000000000000 00000000e96b193f ffff88042c5dbd48 ffffffff81380a5f > ffff88042c5dbd90 ffff88042c5dbd80 ffffffff8109d3f2 80000000000001e1 > 0000000000000003 ffff88042c5dbe90 ffff88042c5dbe90 0000000000000000 > Call Trace: > [<ffffffff81380a5f>] dump_stack+0x44/0x55 > [<ffffffff8109d3f2>] warn_slowpath_common+0x82/0xc0 > [<ffffffff8109d48c>] warn_slowpath_fmt+0x5c/0x80 > [<ffffffff8106cfc9>] ? note_page+0x5c9/0x7b0 > [<ffffffff8106d010>] note_page+0x610/0x7b0 > [<ffffffff8106d409>] ptdump_walk_pgd_level_core+0x259/0x3c0 > [<ffffffff8106d5a7>] ptdump_walk_pgd_level_checkwx+0x17/0x20 > [<ffffffff81063905>] mark_rodata_ro+0xf5/0x100 > [<ffffffff817415a0>] ? rest_init+0x80/0x80 > [<ffffffff817415bd>] kernel_init+0x1d/0xe0 > [<ffffffff8174cd1f>] ret_from_fork+0x3f/0x70 > [<ffffffff817415a0>] ? rest_init+0x80/0x80 > ---[ end trace a1f23a1e42a2ac76 ]--- > x86/mm: Checked W+X mappings: FAILED, 171 W+X pages found. > > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Acked-by: Kees Cook <keescook@chromium.org>
Thanks!
-Kees
> --- > v3 enables the checks on 32-bit if NX is supported, and also > makes DEBUG_WX depend on DEBUG_RODATA since both the NX marking > and the checking occurs from mark_rodata_ro(). > > arch/x86/Kconfig.debug | 20 +++++++++++++++++++- > arch/x86/include/asm/pgtable.h | 7 +++++++ > arch/x86/mm/Makefile | 2 +- > arch/x86/mm/dump_pagetables.c | 42 +++++++++++++++++++++++++++++++++++++++++- > arch/x86/mm/init_32.c | 2 ++ > arch/x86/mm/init_64.c | 2 ++ > 6 files changed, 72 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug > index d8c0d32..d09fde7 100644 > --- a/arch/x86/Kconfig.debug > +++ b/arch/x86/Kconfig.debug > @@ -65,10 +65,14 @@ config EARLY_PRINTK_EFI > This is useful for kernel debugging when your machine crashes very > early before the console code is initialized. > > +config X86_PTDUMP_CORE > + def_bool n > + > config X86_PTDUMP > bool "Export kernel pagetable layout to userspace via debugfs" > depends on DEBUG_KERNEL > select DEBUG_FS > + select X86_PTDUMP_CORE > ---help--- > Say Y here if you want to show the kernel pagetable layout in a > debugfs file. This information is only useful for kernel developers > @@ -79,7 +83,8 @@ config X86_PTDUMP > > config EFI_PGT_DUMP > bool "Dump the EFI pagetable" > - depends on EFI && X86_PTDUMP > + depends on EFI > + select X86_PTDUMP_CORE > ---help--- > Enable this if you want to dump the EFI page table before > enabling virtual mode. This can be used to debug miscellaneous > @@ -105,6 +110,19 @@ config DEBUG_RODATA_TEST > feature as well as for the change_page_attr() infrastructure. > If in doubt, say "N" > > +config DEBUG_WX > + bool "Warn on W+X mappings at boot" > + depends on DEBUG_RODATA > + select X86_PTDUMP_CORE > + ---help--- > + Generate a warning if any W+X mappings are found at boot. > + This is useful for discovering cases where the kernel is leaving > + W+X mappings after applying NX, as such mappings are a security risk. > + Look for a message in dmesg output like this: > + x86/mm: Checked W+X mappings: passed, no W+X pages found. > + or like this: > + x86/mm: Checked W+X mappings: FAILED, <N> W+X pages found. > + > config DEBUG_SET_MODULE_RONX > bool "Set loadable kernel module data as NX and text as RO" > depends on MODULES > diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h > index 867da5b..f2b6bed 100644 > --- a/arch/x86/include/asm/pgtable.h > +++ b/arch/x86/include/asm/pgtable.h > @@ -19,6 +19,13 @@ > #include <asm/x86_init.h> > > void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd); > +void ptdump_walk_pgd_level_checkwx(void); > + > +#ifdef CONFIG_DEBUG_WX > +#define debug_checkwx() ptdump_walk_pgd_level_checkwx() > +#else > +#define debug_checkwx() do { } while (0) > +#endif > > /* > * ZERO_PAGE is a global shared page that is always zero: used > diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile > index a482d10..65c47fd 100644 > --- a/arch/x86/mm/Makefile > +++ b/arch/x86/mm/Makefile > @@ -14,7 +14,7 @@ obj-$(CONFIG_SMP) += tlb.o > obj-$(CONFIG_X86_32) += pgtable_32.o iomap_32.o > > obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o > -obj-$(CONFIG_X86_PTDUMP) += dump_pagetables.o > +obj-$(CONFIG_X86_PTDUMP_CORE) += dump_pagetables.o > > obj-$(CONFIG_HIGHMEM) += highmem_32.o > > diff --git a/arch/x86/mm/dump_pagetables.c b/arch/x86/mm/dump_pagetables.c > index f0cedf3..19c64af 100644 > --- a/arch/x86/mm/dump_pagetables.c > +++ b/arch/x86/mm/dump_pagetables.c > @@ -32,6 +32,8 @@ struct pg_state { > const struct addr_marker *marker; > unsigned long lines; > bool to_dmesg; > + bool check_wx; > + unsigned long wx_pages; > }; > > struct addr_marker { > @@ -214,6 +216,16 @@ static void note_page(struct seq_file *m, struct pg_state *st, > const char *unit = units; > unsigned long delta; > int width = sizeof(unsigned long) * 2; > + pgprotval_t pr = pgprot_val(st->current_prot); > + > + if (st->check_wx && (pr & _PAGE_RW) && !(pr & _PAGE_NX)) { > + WARN_ONCE(1, > + "x86/mm: Found insecure W+X mapping at address %p/%pS\n", > + (void *)st->start_address, > + (void *)st->start_address); > + st->wx_pages += (st->current_address - > + st->start_address) / PAGE_SIZE; > + } > > /* > * Now print the actual finished series > @@ -344,7 +356,8 @@ static void walk_pud_level(struct seq_file *m, struct pg_state *st, pgd_t addr, > #define pgd_none(a) pud_none(__pud(pgd_val(a))) > #endif > > -void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd) > +static void ptdump_walk_pgd_level_core(struct seq_file *m, pgd_t *pgd, > + bool checkwx) > { > #ifdef CONFIG_X86_64 > pgd_t *start = (pgd_t *) &init_level4_pgt; > @@ -359,6 +372,10 @@ void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd) > st.to_dmesg = true; > } > > + st.check_wx = checkwx; > + if (checkwx) > + st.wx_pages = 0; > + > for (i = 0; i < PTRS_PER_PGD; i++) { > st.current_address = normalize_addr(i * PGD_LEVEL_MULT); > if (!pgd_none(*start)) { > @@ -378,8 +395,26 @@ void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd) > /* Flush out the last page */ > st.current_address = normalize_addr(PTRS_PER_PGD*PGD_LEVEL_MULT); > note_page(m, &st, __pgprot(0), 0); > + if (!checkwx) > + return; > + if (st.wx_pages) > + pr_info("x86/mm: Checked W+X mappings: FAILED, %lu W+X pages found.\n", > + st.wx_pages); > + else > + pr_info("x86/mm: Checked W+X mappings: passed, no W+X pages found.\n"); > +} > + > +void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd) > +{ > + ptdump_walk_pgd_level_core(m, pgd, false); > } > > +void ptdump_walk_pgd_level_checkwx(void) > +{ > + ptdump_walk_pgd_level_core(NULL, NULL, true); > +} > + > +#ifdef CONFIG_X86_PTDUMP > static int ptdump_show(struct seq_file *m, void *v) > { > ptdump_walk_pgd_level(m, NULL); > @@ -397,10 +432,13 @@ static const struct file_operations ptdump_fops = { > .llseek = seq_lseek, > .release = single_release, > }; > +#endif > > static int pt_dump_init(void) > { > +#ifdef CONFIG_X86_PTDUMP > struct dentry *pe; > +#endif > > #ifdef CONFIG_X86_32 > /* Not a compile-time constant on x86-32 */ > @@ -412,10 +450,12 @@ static int pt_dump_init(void) > address_markers[FIXADDR_START_NR].start_address = FIXADDR_START; > #endif > > +#ifdef CONFIG_X86_PTDUMP > pe = debugfs_create_file("kernel_page_tables", 0600, NULL, NULL, > &ptdump_fops); > if (!pe) > return -ENOMEM; > +#endif > > return 0; > } > diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c > index 7562f42..cb4ef3d 100644 > --- a/arch/x86/mm/init_32.c > +++ b/arch/x86/mm/init_32.c > @@ -957,6 +957,8 @@ void mark_rodata_ro(void) > set_pages_ro(virt_to_page(start), size >> PAGE_SHIFT); > #endif > mark_nxdata_nx(); > + if (__supported_pte_mask & _PAGE_NX) > + debug_checkwx(); > } > #endif > > diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c > index df48430..5ed62ef 100644 > --- a/arch/x86/mm/init_64.c > +++ b/arch/x86/mm/init_64.c > @@ -1150,6 +1150,8 @@ void mark_rodata_ro(void) > free_init_pages("unused kernel", > (unsigned long) __va(__pa_symbol(rodata_end)), > (unsigned long) __va(__pa_symbol(_sdata))); > + > + debug_checkwx(); > } > > #endif > -- > 2.1.0 >
-- Kees Cook Chrome OS Security
| |