Commit e1a58320 authored by Stephen Smalley's avatar Stephen Smalley Committed by Ingo Molnar

x86/mm: Warn on W^X mappings

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 it prints this to the kernel log:

  x86/mm: Checked W+X mappings: passed, no W+X pages found.

On failure it prints a warning and a count of the failed pages:

  ------------[ 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
  [...]
  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: default avatarStephen Smalley <sds@tycho.nsa.gov>
Acked-by: default avatarKees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Link: http://lkml.kernel.org/r/1444064120-11450-1-git-send-email-sds@tycho.nsa.gov
[ Improved the Kconfig help text and made the new option default-y
  if CONFIG_DEBUG_RODATA=y, because it already found buggy mappings,
  so we really want people to have this on by default. ]
Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent 38a413cb
...@@ -65,10 +65,14 @@ config EARLY_PRINTK_EFI ...@@ -65,10 +65,14 @@ config EARLY_PRINTK_EFI
This is useful for kernel debugging when your machine crashes very This is useful for kernel debugging when your machine crashes very
early before the console code is initialized. early before the console code is initialized.
config X86_PTDUMP_CORE
def_bool n
config X86_PTDUMP config X86_PTDUMP
bool "Export kernel pagetable layout to userspace via debugfs" bool "Export kernel pagetable layout to userspace via debugfs"
depends on DEBUG_KERNEL depends on DEBUG_KERNEL
select DEBUG_FS select DEBUG_FS
select X86_PTDUMP_CORE
---help--- ---help---
Say Y here if you want to show the kernel pagetable layout in a Say Y here if you want to show the kernel pagetable layout in a
debugfs file. This information is only useful for kernel developers debugfs file. This information is only useful for kernel developers
...@@ -79,7 +83,8 @@ config X86_PTDUMP ...@@ -79,7 +83,8 @@ config X86_PTDUMP
config EFI_PGT_DUMP config EFI_PGT_DUMP
bool "Dump the EFI pagetable" bool "Dump the EFI pagetable"
depends on EFI && X86_PTDUMP depends on EFI
select X86_PTDUMP_CORE
---help--- ---help---
Enable this if you want to dump the EFI page table before Enable this if you want to dump the EFI page table before
enabling virtual mode. This can be used to debug miscellaneous enabling virtual mode. This can be used to debug miscellaneous
...@@ -105,6 +110,35 @@ config DEBUG_RODATA_TEST ...@@ -105,6 +110,35 @@ config DEBUG_RODATA_TEST
feature as well as for the change_page_attr() infrastructure. feature as well as for the change_page_attr() infrastructure.
If in doubt, say "N" If in doubt, say "N"
config DEBUG_WX
bool "Warn on W+X mappings at boot"
depends on DEBUG_RODATA
default y
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, if the check failed:
x86/mm: Checked W+X mappings: FAILED, <N> W+X pages found.
Note that even if the check fails, your kernel is possibly
still fine, as W+X mappings are not a security hole in
themselves, what they do is that they make the exploitation
of other unfixed kernel bugs easier.
There is no runtime or memory usage effect of this option
once the kernel has booted up - it's a one time check.
If in doubt, say "Y".
config DEBUG_SET_MODULE_RONX config DEBUG_SET_MODULE_RONX
bool "Set loadable kernel module data as NX and text as RO" bool "Set loadable kernel module data as NX and text as RO"
depends on MODULES depends on MODULES
......
...@@ -19,6 +19,13 @@ ...@@ -19,6 +19,13 @@
#include <asm/x86_init.h> #include <asm/x86_init.h>
void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd); 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 * ZERO_PAGE is a global shared page that is always zero: used
......
...@@ -14,7 +14,7 @@ obj-$(CONFIG_SMP) += tlb.o ...@@ -14,7 +14,7 @@ obj-$(CONFIG_SMP) += tlb.o
obj-$(CONFIG_X86_32) += pgtable_32.o iomap_32.o obj-$(CONFIG_X86_32) += pgtable_32.o iomap_32.o
obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.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 obj-$(CONFIG_HIGHMEM) += highmem_32.o
......
...@@ -32,6 +32,8 @@ struct pg_state { ...@@ -32,6 +32,8 @@ struct pg_state {
const struct addr_marker *marker; const struct addr_marker *marker;
unsigned long lines; unsigned long lines;
bool to_dmesg; bool to_dmesg;
bool check_wx;
unsigned long wx_pages;
}; };
struct addr_marker { struct addr_marker {
...@@ -214,6 +216,16 @@ static void note_page(struct seq_file *m, struct pg_state *st, ...@@ -214,6 +216,16 @@ static void note_page(struct seq_file *m, struct pg_state *st,
const char *unit = units; const char *unit = units;
unsigned long delta; unsigned long delta;
int width = sizeof(unsigned long) * 2; 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 * Now print the actual finished series
...@@ -346,7 +358,8 @@ static void walk_pud_level(struct seq_file *m, struct pg_state *st, pgd_t addr, ...@@ -346,7 +358,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))) #define pgd_none(a) pud_none(__pud(pgd_val(a)))
#endif #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 #ifdef CONFIG_X86_64
pgd_t *start = (pgd_t *) &init_level4_pgt; pgd_t *start = (pgd_t *) &init_level4_pgt;
...@@ -362,6 +375,10 @@ void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd) ...@@ -362,6 +375,10 @@ void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd)
st.to_dmesg = true; st.to_dmesg = true;
} }
st.check_wx = checkwx;
if (checkwx)
st.wx_pages = 0;
for (i = 0; i < PTRS_PER_PGD; i++) { for (i = 0; i < PTRS_PER_PGD; i++) {
st.current_address = normalize_addr(i * PGD_LEVEL_MULT); st.current_address = normalize_addr(i * PGD_LEVEL_MULT);
if (!pgd_none(*start)) { if (!pgd_none(*start)) {
...@@ -381,8 +398,26 @@ void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd) ...@@ -381,8 +398,26 @@ void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd)
/* Flush out the last page */ /* Flush out the last page */
st.current_address = normalize_addr(PTRS_PER_PGD*PGD_LEVEL_MULT); st.current_address = normalize_addr(PTRS_PER_PGD*PGD_LEVEL_MULT);
note_page(m, &st, __pgprot(0), 0); 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) static int ptdump_show(struct seq_file *m, void *v)
{ {
ptdump_walk_pgd_level(m, NULL); ptdump_walk_pgd_level(m, NULL);
...@@ -400,10 +435,13 @@ static const struct file_operations ptdump_fops = { ...@@ -400,10 +435,13 @@ static const struct file_operations ptdump_fops = {
.llseek = seq_lseek, .llseek = seq_lseek,
.release = single_release, .release = single_release,
}; };
#endif
static int pt_dump_init(void) static int pt_dump_init(void)
{ {
#ifdef CONFIG_X86_PTDUMP
struct dentry *pe; struct dentry *pe;
#endif
#ifdef CONFIG_X86_32 #ifdef CONFIG_X86_32
/* Not a compile-time constant on x86-32 */ /* Not a compile-time constant on x86-32 */
...@@ -415,10 +453,12 @@ static int pt_dump_init(void) ...@@ -415,10 +453,12 @@ static int pt_dump_init(void)
address_markers[FIXADDR_START_NR].start_address = FIXADDR_START; address_markers[FIXADDR_START_NR].start_address = FIXADDR_START;
#endif #endif
#ifdef CONFIG_X86_PTDUMP
pe = debugfs_create_file("kernel_page_tables", 0600, NULL, NULL, pe = debugfs_create_file("kernel_page_tables", 0600, NULL, NULL,
&ptdump_fops); &ptdump_fops);
if (!pe) if (!pe)
return -ENOMEM; return -ENOMEM;
#endif
return 0; return 0;
} }
......
...@@ -957,6 +957,8 @@ void mark_rodata_ro(void) ...@@ -957,6 +957,8 @@ void mark_rodata_ro(void)
set_pages_ro(virt_to_page(start), size >> PAGE_SHIFT); set_pages_ro(virt_to_page(start), size >> PAGE_SHIFT);
#endif #endif
mark_nxdata_nx(); mark_nxdata_nx();
if (__supported_pte_mask & _PAGE_NX)
debug_checkwx();
} }
#endif #endif
...@@ -1150,6 +1150,8 @@ void mark_rodata_ro(void) ...@@ -1150,6 +1150,8 @@ void mark_rodata_ro(void)
free_init_pages("unused kernel", free_init_pages("unused kernel",
(unsigned long) __va(__pa_symbol(rodata_end)), (unsigned long) __va(__pa_symbol(rodata_end)),
(unsigned long) __va(__pa_symbol(_sdata))); (unsigned long) __va(__pa_symbol(_sdata)));
debug_checkwx();
} }
#endif #endif
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment