linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matt Fleming <matt@codeblueprint.co.uk>
To: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>,
	Stephen Smalley <sds@tycho.nsa.gov>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	keescook@chromium.org, Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Andy Lutomirski <luto@kernel.org>,
	Denys Vlasenko <dvlasenk@redhat.com>,
	Brian Gerst <brgerst@gmail.com>,
	linux-efi@vger.kernel.org,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [PATCH v2] x86/mm: warn on W+x mappings
Date: Wed, 21 Oct 2015 21:38:07 +0100	[thread overview]
Message-ID: <20151021203807.GA20338@codeblueprint.co.uk> (raw)
In-Reply-To: <20151021094242.GA12155@gmail.com>

On Wed, 21 Oct, at 11:42:42AM, Ingo Molnar wrote:
> 
> * Matt Fleming <matt@codeblueprint.co.uk> wrote:
> 
> > > > Right, we could do that, but then we wouldn't be able to support 
> > > > creation/updating variables at runtime, such as when you install a 
> > > > distribution for the first time, or want to boot a new kernel filename 
> > > > directly from the firmware without a boot loader (and need to modify the 
> > > > BootXXXX variables).
> > > 
> > > Do we know the precise position and address range of these variables?
> > > 
> > > We could map them writable (but not executable), and the rest executable (but 
> > > not writable).
> >  
> > The variables are stored in NVRAM, which we don't map into the kernel virtual 
> > address space. [...]
> 
> Just curious: is there firmware that memory maps those variables privately?
 
Good question, not sure. I suspect not because it becomes much harder
to protect those oh-so-precious variables from errant code wanting to
write to them.

Usually things get written on x86 from SMM code.

> > [...] We have to initiate the transaction of writing to the variables by 
> > executing EFI runtime services.
> > 
> > We obviously have buffers that we pass to the BIOS that contain variable data, 
> > but these should be NX anyway because they're regular kernel allocations.
> > 
> > > That raises the question whether the same physical page ever mixes variables 
> > > and actual code - but the hope would be that it's suffiently page granular for 
> > > this to work.
> > 
> > I don't think that would ever happen.
> 
> Ok, that's promising, so how about this then to solve the security weakness the 
> new warning unearthed: map the whole EFI range as 'r-x (NX)', but detect writes 
> from the page fault handler and transparently allow them to flip over the range to 
> 'rw-'.
> 
> Note that for security reasons we don't allow a subsequent flipping back to NX if 
> there's an NX fault on the same page, i.e. this new mechanism is a monotonic 
> one-way process that should dynamically 'map out' data pages versus executable 
> pages.
> 
> It should also be pretty robust, assuming we can take page faults while EFI code 
> is executing and is trying to modify EFI data: is that the case?

Yes, we can do that but I think I misunderstood what you were asking
when you said,

 > That raises the question whether the same physical page ever mixes variables and
 > actual code - but the hope would be that it's suffiently page granular for this to
 > work.
 
I was talking about EFI variables as defined in the UEFI spec, i.e.
backed by some peristent storage mechanism. I wasn't talking about
".data" objects.

It *is* possible for physical pages to contain both EFI code and data,
as Ard mentioned, and we have no way of distinguishing when EFI code
tried to write to a EfiRuntimeServicesCode page/region because there's
also legitimate data there and when an exploit attempt is taking
place.

In which case, I think we'd essentially map everything with execute
permission apart from the heap and other dynamically allocated objects
stored in EfiRuntimeServicesData.

But at that point, can't we just leave all these regions unmapped
unless we're in the EFI code paths? And that includes not leaving the
mappings around duing the suspend/resume code. 

  parent reply	other threads:[~2015-10-21 20:38 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-02 19:29 [PATCH v2] x86/mm: warn on W+x mappings Stephen Smalley
2015-10-02 20:44 ` Kees Cook
2015-10-03 11:27 ` Ingo Molnar
2015-10-05 19:13   ` Stephen Smalley
2015-10-06  7:32     ` Ingo Molnar
2015-10-06 15:37       ` Stephen Smalley
2015-10-12 11:36         ` Borislav Petkov
2015-10-12 12:41           ` Matt Fleming
2015-10-12 12:49             ` Ingo Molnar
2015-10-12 12:55               ` Matt Fleming
2015-10-12 14:17                 ` Ingo Molnar
2015-10-12 14:49                   ` Matt Fleming
2015-10-12 15:34                     ` Ard Biesheuvel
2015-10-12 15:50                       ` Matt Fleming
2015-10-12 16:43                         ` Ard Biesheuvel
2015-10-14 15:18                     ` Ingo Molnar
2015-10-14 15:30                       ` Andy Lutomirski
2015-10-14 15:35                         ` Borislav Petkov
2015-10-15 10:10                           ` Matt Fleming
2015-10-15 10:33                             ` Borislav Petkov
2015-10-16  1:45                               ` Ricardo Neri
2015-10-14 21:02                       ` Matt Fleming
2015-10-21  9:42                         ` Ingo Molnar
2015-10-21 12:49                           ` Ingo Molnar
2015-10-21 12:57                             ` Ard Biesheuvel
2015-10-21 13:24                               ` Borislav Petkov
2015-10-21 13:28                                 ` Ard Biesheuvel
2015-10-21 14:36                                   ` Borislav Petkov
2015-10-21 18:46                                     ` Andy Lutomirski
2015-10-21 20:45                                       ` Matt Fleming
2015-10-21 20:49                                         ` Andy Lutomirski
2015-10-21 20:38                           ` Matt Fleming [this message]
2015-10-12 14:56                   ` Josh Triplett
2015-10-14 15:19                     ` Ingo Molnar
2015-10-14 16:47                       ` Josh Triplett
2015-10-21  9:43                         ` Ingo Molnar

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=20151021203807.GA20338@codeblueprint.co.uk \
    --to=matt@codeblueprint.co.uk \
    --cc=a.p.zijlstra@chello.nl \
    --cc=ard.biesheuvel@linaro.org \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=dvlasenk@redhat.com \
    --cc=hpa@zytor.com \
    --cc=keescook@chromium.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=sds@tycho.nsa.gov \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.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).