Messages in this thread | | | Date | Thu, 1 Oct 2015 12:24:25 -0700 | Subject | Re: [RFC][PATCH] x86/mm: warn on W+x mappings | From | Kees Cook <> |
| |
On Thu, Oct 1, 2015 at 9:28 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > Warn on any residual W+x mappings if X86_PTDUMP is enabled. > > Sample dmesg output: > Checking for W+x mappings > 0xffffffff81755000-0xffffffff81800000 684K RW GLB x pte > Found W+x mappings. Please fix. > > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> > --- > Not sure if this is the best place to put this check. > It must occur after free_init_pages() or it won't catch the > W+x case for the gap between __ex_table and rodata.
Yeah. Hmm. I want this test for sure, but I'd like to be able to do with without needing PTDUMP, since that puts a very sensitive file in debugfs. I wonder if we can reuse the same code, but only expose the page tables to userspace with PTDUMP?
-Kees
> > arch/x86/include/asm/pgtable.h | 6 ++++++ > arch/x86/mm/dump_pagetables.c | 31 ++++++++++++++++++++++++++++++- > arch/x86/mm/init_64.c | 2 ++ > 3 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h > index 867da5b..8e771c1 100644 > --- a/arch/x86/include/asm/pgtable.h > +++ b/arch/x86/include/asm/pgtable.h > @@ -20,6 +20,12 @@ > > void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd); > > +#ifdef CONFIG_X86_PTDUMP > +void ptdump_walk_pgd_level_checkwx(void); > +#else > +#define ptdump_walk_pgd_level_checkwx() do { } while (0) > +#endif > + > /* > * ZERO_PAGE is a global shared page that is always zero: used > * for zero-mapped memory areas etc.. > diff --git a/arch/x86/mm/dump_pagetables.c b/arch/x86/mm/dump_pagetables.c > index f0cedf3..986903b 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; > + bool found_wx; > }; > > struct addr_marker { > @@ -214,6 +216,13 @@ 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); > + bool savedmesg = st->to_dmesg; > + > + if (st->check_wx && (pr & _PAGE_RW) && !(pr & _PAGE_NX)) { > + st->to_dmesg = true; > + st->found_wx = true; > + } > > /* > * Now print the actual finished series > @@ -261,6 +270,7 @@ static void note_page(struct seq_file *m, struct pg_state *st, > st->start_address = st->current_address; > st->current_prot = new_prot; > st->level = level; > + st->to_dmesg = savedmesg; > } > } > > @@ -344,7 +354,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 +370,12 @@ void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd) > st.to_dmesg = true; > } > > + st.check_wx = checkwx; > + if (checkwx) { > + pr_info("Checking for W+x mappings\n"); > + st.found_wx = false; > + } > + > for (i = 0; i < PTRS_PER_PGD; i++) { > st.current_address = normalize_addr(i * PGD_LEVEL_MULT); > if (!pgd_none(*start)) { > @@ -378,6 +395,18 @@ 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 && st.found_wx) > + pr_warn("Found W+x mappings. Please fix.\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); > } > > static int ptdump_show(struct seq_file *m, void *v) > diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c > index df48430..7e704da 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))); > + > + ptdump_walk_pgd_level_checkwx(); > } > > #endif > -- > 2.1.0 >
-- Kees Cook Chrome OS Security
| |