From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1751428AbbJBHhO (ORCPT <rfc822;w@1wt.eu>);
	Fri, 2 Oct 2015 03:37:14 -0400
Received: from mail-wi0-f169.google.com ([209.85.212.169]:37045 "EHLO
	mail-wi0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1750962AbbJBHhN (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 2 Oct 2015 03:37:13 -0400
Date: Fri, 2 Oct 2015 09:37:09 +0200
From: Ingo Molnar <mingo@kernel.org>
To: Stephen Smalley <sds@tycho.nsa.gov>
Cc: x86@kernel.org, linux-kernel@vger.kernel.org, keescook@chromium.org,
        Borislav Petkov <bp@alien8.de>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Andy Lutomirski <luto@kernel.org>,
        Denys Vlasenko <dvlasenk@redhat.com>, Brian Gerst <brgerst@gmail.com>
Subject: Re: [RFC][PATCH] x86/mm: warn on W+x mappings
Message-ID: <20151002073708.GB5035@gmail.com>
References: <1443716923-6072-1-git-send-email-sds@tycho.nsa.gov>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1443716923-6072-1-git-send-email-sds@tycho.nsa.gov>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


* Stephen Smalley <sds@tycho.nsa.gov> wrote:

> +	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");

So I like the patch, except the way it presents failures:

please generate any printks() only after the check has been performed, not before 
it. There should be a single line printed in the 'test successful' case:

	pr_info("x86/mm: Checked W+X mappings: passed, no W+X pages found.\n");

In the failure case there should be printk()s generated at the first point of 
violation:

+               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;
+               }

via something like:

	WARN_ONCE(1, "x86/mm: Found insecure W+X mapping at address %p/%pS\n", addr, addr);

i.e. output the address and the symbolic name as well. Only generate a single 
warning - we expect the normal kernel to be entirely warnings-free.

and then print the number of pages with bad mappings in the 'result' line:

	pr_info("x86/mm: Checked W+X mappings: FAILED, %ld W+X pages found.\n", nr_failures);

Thanks,

	Ingo