linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Kees Cook <keescook@chromium.org>
Cc: Borislav Petkov <bp@suse.de>, Ingo Molnar <mingo@kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>, Pavel Machek <pavel@ucw.cz>,
	Linux PM list <linux-pm@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	shuzzle@mailbox.org, Thomas Garnier <thgarnie@google.com>
Subject: Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk
Date: Tue, 26 Jul 2016 22:53:20 +0200	[thread overview]
Message-ID: <2264716.77pSArDZvG@vostro.rjw.lan> (raw)
In-Reply-To: <CAGXu5jLEbETSntwVQnNt4MZsHiU3E0OCN51VgBScvVAjCP7auA@mail.gmail.com>

On Tuesday, July 26, 2016 01:33:02 PM Kees Cook wrote:
> On Tue, Jul 26, 2016 at 1:24 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Tuesday, July 26, 2016 04:04:42 PM Borislav Petkov wrote:
> >> On Tue, Jul 26, 2016 at 01:32:28PM +0200, Rafael J. Wysocki wrote:
> >> > Hi,
> >> >
> >> > The following commit:
> >> >
> >> > commit 13523309495cdbd57a0d344c0d5d574987af007f
> >> > Author: Josh Poimboeuf <jpoimboe@redhat.com>
> >> > Date:   Thu Jan 21 16:49:21 2016 -0600
> >> >
> >> >     x86/asm/acpi: Create a stack frame in do_suspend_lowlevel()
> >> >
> >> >     do_suspend_lowlevel() is a callable non-leaf function which doesn't
> >> >     honor CONFIG_FRAME_POINTER, which can result in bad stack traces.
> >> >
> >> >     Create a stack frame for it when CONFIG_FRAME_POINTER is enabled.
> >> >
> >> > is reported to cause a resume-from-hibernation regression due to an attempt
> >> > to execute an NX page (we've seen quite a bit of that recently).
> >> >
> >> > I'm asking the reporter to try 4.7, but if the problem is still there, we'll
> >> > need to revert the above I'm afraid.
> >>
> >> So I can't resume properly from disk too, on the Intel laptop this time. Top
> >> commit is from tip/master:
> >>
> >> commit 516f48acf59722429acd323b3d283f74f02891fe (refs/remotes/tip/master)
> >> Merge: a4823bbffc96 dd9506954539
> >> Author: Ingo Molnar <mingo@kernel.org>
> >> Date:   Mon Jul 25 08:39:43 2016 +0200
> >>
> >>     Merge branch 'linus'
> >>
> >>
> >> So I thought it might be Josh's patch above and reverted it. No joy.
> >>
> >> Then I remembered that I enabled CONFIG_RANDOMIZE_MEMORY for the
> >> microcode loader breakage which we've been debugging. Turned that off
> >> and machine resumes fine again.
> >
> > Well, I wasn't aware of *another* flavor of ASLR in the works.  And there
> > was no hope it would not break hibernation if you asked me.
> >
> >> It looks like
> >>
> >>   0483e1fa6e09 ("x86/mm: Implement ASLR for kernel memory regions")
> >>
> >> broke a bunch of things. Off the top of my head, we probably should make
> >> suspend to disk and CONFIG_RANDOMIZE_MEMORY mutually exclusive, like it
> >> was the case with ASLR previously, AFAIR.
> >
> > Please no.
> >
> > First off, it should be perfectly possible to make hibernation work along
> > with this new variant of ASLR.  Second, quite obviously, the author of these
> > ASLR changes had not done sufficient research to estimate the possible
> > impact of them.
> 
> I think that's a bit unfair: Thomas did a lot of testing, and it has
> been living in -next for a while.

Well, with all due respect, "a lot of testing" is not quite the same thing as
"sufficient research" IMO.

It should be known (at least from experience) that hibernation on x86-64 doesn't
play well with ASLR quite as a rule, so it would be good to at least check that
particular thing or CC a relevant person (ie. me).

Or even ask me on IRC for that matter.  Give me a heads up ahead of time.

But no.  I'm still on the receiving end of the "hibernation doesn't work with
ASLR" story which was entirely avoidable this time around.  Sigh.

> > Honestly, I don't think it is a good idea to introduce random Kconfig options
> > for working around cases in which the author of some changes cannot be bothered
> > with doing things right.  Even if that is security.
> 
> I would agree: let's try to get this fixed soon.
> 
> > So IMO, either we should fix the problem, or that whole new ASLR stuff should
> > be reverted.
> >
> > I think I know how to fix it, but I won't be able to get to that before the
> > next week.  I guess it can wait till then, though.
> 
> Thomas, will you have some time to examine this and estimate the work for a fix?

FWIW, my hunch ATM is that you need to look at the "Set up the direct mapping
from scratch" loop in set_up_temporary_mappings() and make it do the right
thing when the new ASLR stuff is enabled.

Thanks,
Rafael

  reply	other threads:[~2016-07-26 20:48 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-26 11:32 Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk Rafael J. Wysocki
2016-07-26 14:04 ` Borislav Petkov
2016-07-26 20:24   ` Rafael J. Wysocki
2016-07-26 20:33     ` Kees Cook
2016-07-26 20:53       ` Rafael J. Wysocki [this message]
2016-07-26 20:59         ` Kees Cook
2016-07-26 21:17           ` Thomas Garnier
2016-07-27  5:39             ` Borislav Petkov
2016-07-26 14:39 ` Josh Poimboeuf
2016-07-26 20:15   ` Rafael J. Wysocki
2016-07-26 20:31     ` Kees Cook
2016-07-26 20:42       ` Rafael J. Wysocki
2016-07-26 21:53     ` Josh Poimboeuf
2016-07-26 22:42       ` Rafael J. Wysocki
2016-07-26 23:08         ` Rafael J. Wysocki
2016-07-27 17:59           ` Josh Poimboeuf
2016-07-27 22:12             ` Rafael J. Wysocki
2016-07-27 22:17               ` Josh Poimboeuf
2016-07-27 23:20                 ` Rafael J. Wysocki
2016-07-27 23:29                   ` Rafael J. Wysocki
2016-07-28 15:17                     ` [PATCH] x86/asm/power: Fix hibernation return address corruption Josh Poimboeuf
2016-07-28 15:32                       ` Josh Poimboeuf
2016-07-28 21:36                       ` Rafael J. Wysocki
2016-07-29  7:16                         ` Ingo Molnar
2016-07-27 22:20               ` Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk Rafael J. Wysocki

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=2264716.77pSArDZvG@vostro.rjw.lan \
    --to=rjw@rjwysocki.net \
    --cc=bp@suse.de \
    --cc=jpoimboe@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=pavel@ucw.cz \
    --cc=shuzzle@mailbox.org \
    --cc=tglx@linutronix.de \
    --cc=thgarnie@google.com \
    /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).