linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: matthieu castet <castet.matthieu@free.fr>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>,
	Kees Cook <kees.cook@canonical.com>,
	Jeremy Fitzhardinge <jeremy@goop.org>,
	"keir.fraser@eu.citrix.com" <keir.fraser@eu.citrix.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"sliakh.lkml@gmail.com" <sliakh.lkml@gmail.com>,
	"jmorris@namei.org" <jmorris@namei.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"rusty@rustcorp.com.au" <rusty@rustcorp.com.au>,
	"torvalds@linux-foundation.org" <torvalds@linux-foundation.org>,
	"ak@muc.de" <ak@muc.de>, "davej@redhat.com" <davej@redhat.com>,
	"jiang@cs.ncsu.edu" <jiang@cs.ncsu.edu>,
	"arjan@infradead.org" <arjan@infradead.org>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"sfr@canb.auug.org.au" <sfr@canb.auug.org.au>,
	"mingo@elte.hu" <mingo@elte.hu>,
	Stefan Bader <stefan.bader@canonical.com>
Subject: Re: [tip:x86/security] x86: Add NX protection for kernel data
Date: Fri, 21 Jan 2011 18:20:12 -0500	[thread overview]
Message-ID: <20110121232012.GA17652@dumpdata.com> (raw)
In-Reply-To: <20110120215556.GA29700@dumpdata.com>


So this patch fixes the regression and allows to boot Linux on Xen.
It does not affect negatively baremetal (tried x86_32 and x86_64).

The debugfs pagetable looks exactly as before this patch (from
__init_end and to further it expands as time goes on):
0xc14e7000-0xccc00000      187492K     RW             GLB NX pte.

I am concerned with 64edc8ed5ffae999d8d413ba006850e9e34166cb
(x86: Fix improper large page preservation) as it would make it possible
for the .bss sections to have RO pages in it - which would inhibit
the kernel from collapsing the small pages into a large one. However on
the machines I boot this patched kernel up it had no trouble
collapsing small pages in a large one (it also did not have
any pages in .bss section set to RO). Are there any known
(besides Xen) sections of code that sets pages in .bss and .data to RO?

commit 0390f0a87470fd2686c4eed6ace28443f8cc9b2f
Author: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date:   Fri Jan 21 11:23:39 2011 -0500

    x86: Don't force in .bss everything to RW.
    
    If there are pages there which are RO leave them be. Otherwise
    set them to RW.
    
    This fixes a bug where under Xen we would get:
    (XEN) mm.c:2389:d0 Bad type (saw 3c000003 != exp 70000000) for mfn 1335a1 (pfn 15a1)
    (XEN) mm.c:889:d0 Error getting mfn 1335a1 (pfn 15a1) from L1 entry 80000001335a1063 for l1e_owner=0, pg_owner=0
    (XEN) mm.c:4939:d0 ptwr_emulate: could not get_page_from_l1e()
    [    8.296405] BUG: unable to handle kernel paging request at cb159d08
    [    8.296405] IP: [<c1005ff4>] xen_set_pte_atomic+0x21/0x2f
    [    8.296405] *pdpt = 000000000165e001 *pde = 000000000b1a7067 *pte = 800000000b159061
    
    B/c we tried to set RW on the swapper_pg_dir, which had been
    set to RO by the Xen init code (xen_write_cr3_init) and MUST be
    RO (so that the Xen hypervisor can be assured that the guest is not
    messing with the pagetables on its own).
    
    Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 8b830ca..53b99e3 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -283,11 +283,18 @@ static inline pgprot_t static_protections(pgprot_t prot, unsigned long address,
 		   __pa((unsigned long)__end_rodata) >> PAGE_SHIFT))
 		pgprot_val(forbidden) |= _PAGE_RW;
 	/*
-	 * .data and .bss should always be writable.
+	 * .data and .bss ought to be writable. But if there is a RO
+	 * in there, don't force RW on it.
 	 */
 	if (within(address, (unsigned long)_sdata, (unsigned long)_edata) ||
-	    within(address, (unsigned long)__bss_start, (unsigned long)__bss_stop))
-		pgprot_val(required) |= _PAGE_RW;
+	    within(address, (unsigned long)__bss_start, (unsigned long)__bss_stop)) {
+		unsigned int level;
+		pte_t *pte = lookup_address(address, &level);
+		pgprot_t prot = ((pte) ? pte_pgprot(*pte) : __pgprot(0));
+
+		if ((pgprot_val(prot) & _PAGE_RW))
+			pgprot_val(required) |= _PAGE_RW;
+	}
 
 #if defined(CONFIG_X86_64) && defined(CONFIG_DEBUG_RODATA)
 	/*
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

      parent reply	other threads:[~2011-01-24 13:57 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-16 21:31 [PATCH 2/3 V8] NX protection for kernel data matthieu castet
2010-11-18 14:08 ` [tip:x86/security] x86: Add " tip-bot for Matthieu Castet
2011-01-11 23:31   ` Kees Cook
2011-01-14 20:15     ` Konrad Rzeszutek Wilk
2011-01-19 21:14       ` Konrad Rzeszutek Wilk
2011-01-19 22:59         ` matthieu castet
2011-01-19 23:38           ` Konrad Rzeszutek Wilk
2011-01-20 11:18             ` castet.matthieu
2011-01-20 15:06               ` Konrad Rzeszutek Wilk
2011-01-20 15:37                 ` Ian Campbell
2011-01-20 19:05                   ` Konrad Rzeszutek Wilk
2011-01-20 20:23                     ` matthieu castet
2011-01-20 21:04                       ` Konrad Rzeszutek Wilk
2011-01-20 21:19                         ` Konrad Rzeszutek Wilk
2011-01-20 21:55                           ` Konrad Rzeszutek Wilk
2011-01-21 21:41                             ` matthieu castet
2011-01-22  5:11                               ` Konrad Rzeszutek Wilk
2011-01-23 14:27                                 ` matthieu castet
2011-01-24 15:31                                   ` Konrad Rzeszutek Wilk
2011-01-27 16:30                                     ` Was: [tip:x86/security] x86: Add NX protection for kernel data. Is: don't set RW on RO regions in .bss Konrad Rzeszutek Wilk
2011-01-21 23:20                             ` Konrad Rzeszutek Wilk [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110121232012.GA17652@dumpdata.com \
    --to=konrad.wilk@oracle.com \
    --cc=Ian.Campbell@eu.citrix.com \
    --cc=ak@muc.de \
    --cc=arjan@infradead.org \
    --cc=castet.matthieu@free.fr \
    --cc=davej@redhat.com \
    --cc=hpa@zytor.com \
    --cc=jeremy@goop.org \
    --cc=jiang@cs.ncsu.edu \
    --cc=jmorris@namei.org \
    --cc=kees.cook@canonical.com \
    --cc=keir.fraser@eu.citrix.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mingo@redhat.com \
    --cc=rusty@rustcorp.com.au \
    --cc=sfr@canb.auug.org.au \
    --cc=sliakh.lkml@gmail.com \
    --cc=stefan.bader@canonical.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).