Re: [tip:x86/mm] x86/mm: Warn on W^X mappings

[Borislav Petkov <bp@alien8.de>]

On Tue, Oct 06, 2015 at 02:54:53AM -0700, tip-bot for Stephen Smalley wrote:
> Commit-ID:  e1a58320a38dfa72be48a0f1a3a92273663ba6db
> Gitweb:     http://git.kernel.org/tip/e1a58320a38dfa72be48a0f1a3a92273663ba6db
> Author:     Stephen Smalley <sds@tycho.nsa.gov>
> AuthorDate: Mon, 5 Oct 2015 12:55:20 -0400
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Tue, 6 Oct 2015 11:11:48 +0200
> 
> 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: Stephen Smalley <sds@tycho.nsa.gov>
> Acked-by: Kees 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: Ingo Molnar <mingo@kernel.org>
> ---
>  arch/x86/Kconfig.debug         | 36 +++++++++++++++++++++++++++++++++++-
>  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, 88 insertions(+), 3 deletions(-)

...

> @@ -381,8 +398,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");

Do we really want to issue anything here in the success case? IMO, we
should be quiet if the check passes and only scream when something's
wrong...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--