Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1450536 > unrolled thread
| Started by | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| First post | 2016-07-26 13:30 +0200 |
| Last post | 2016-07-28 00:20 +0200 |
| Articles | 5 on this page of 25 — 7 participants |
Back to article view | Back to linux.kernel
Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-26 13:30 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk Borislav Petkov <bp@suse.de> - 2016-07-26 16:10 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-26 22:20 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk Kees Cook <keescook@chromium.org> - 2016-07-26 22:40 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-26 22:50 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk Kees Cook <keescook@chromium.org> - 2016-07-26 23:10 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk Thomas Garnier <thgarnie@google.com> - 2016-07-26 23:20 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk Borislav Petkov <bp@suse.de> - 2016-07-27 07:40 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-26 16:40 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-26 22:20 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk Kees Cook <keescook@chromium.org> - 2016-07-26 22:40 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-26 22:40 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-27 00:00 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-27 00:40 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk "Rafael J. Wysocki" <rafael@kernel.org> - 2016-07-27 01:10 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-27 20:00 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-28 00:10 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-28 00:20 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-28 01:20 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-28 01:30 +0200
[PATCH] x86/asm/power: Fix hibernation return address corruption Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-28 17:20 +0200
Re: [PATCH] x86/asm/power: Fix hibernation return address corruption Josh Poimboeuf <jpoimboe@redhat.com> - 2016-07-28 17:40 +0200
Re: [PATCH] x86/asm/power: Fix hibernation return address corruption "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-28 23:40 +0200
Re: [PATCH] x86/asm/power: Fix hibernation return address corruption Ingo Molnar <mingo@kernel.org> - 2016-07-29 09:20 +0200
Re: Fwd: [Bug 150021] New: kernel panic: "kernel tried to execute NX-protected page" when resuming from hibernate to disk "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-28 00:20 +0200
Page 2 of 2 — ← Prev page 1 [2]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-07-28 17:20 +0200 |
| Subject | [PATCH] x86/asm/power: Fix hibernation return address corruption |
| Message-ID | <rZVgl-84L-7@gated-at.bofh.it> |
| In reply to | #1451574 |
On Thu, Jul 28, 2016 at 01:29:49AM +0200, Rafael J. Wysocki wrote:
> On Thursday, July 28, 2016 01:20:53 AM Rafael J. Wysocki wrote:
> > On Wednesday, July 27, 2016 05:17:38 PM Josh Poimboeuf wrote:
> > > On Thu, Jul 28, 2016 at 12:12:15AM +0200, Rafael J. Wysocki wrote:
> > > > On Wednesday, July 27, 2016 12:59:18 PM Josh Poimboeuf wrote:
> > > > > Hm... I have a theory, but I'm not sure about it. I noticed that
> > > > > x86_acpi_enter_sleep_state(),
> > > >
> > > > I think you mean x86_acpi_suspend_lowlevel().
> > >
> > > Oops!
> > >
> > > > > which is involved in suspend, overwrites
> > > > > several global variables (e.g, initial_code) which are used by the CPU
> > > > > boot code in head_64.S. But surprisingly, it doesn't restore those
> > > > > variables to their original values after it resumes.
> > > >
> > > > Is the head_64.S code also used to bring up offline CPUs?
> > >
> > > Yes.
> >
> > OK
> >
> > So it is really interesting why and how that stuff works for everybody.
> >
> > Basically, CPU online should fail after a suspend-resume cycle, but it
> > doesn't most of the time AFAICS.
>
> do_boot_cpu() restores those values, so I think we're safe from that angle.
>
> That should apply to the CPU online during resume from hibernation too.
Yeah, my theory was bogus. And as it turns out, the bug reporter made a
mistake in the bisect. The actual offending commit was apparently:
ef0f3ed5a4ac ("x86/asm/power: Create stack frames in hibernate_asm_64.S")
Amazingly enough, I authored that patch as well. I think "git bisect"
doesn't like me!
Here's the fix:
----
From: Josh Poimboeuf <jpoimboe@redhat.com>
Subject: [PATCH] x86/asm/power: Fix hibernation return address corruption
In kernel bug 150021, a kernel panic was reported when restoring a
hibernate image. Only a picture of the oops was reported, so I can't
paste the whole thing here. But here are the most interesting parts:
kernel tried to execute NX-protected page - exploit attempt? (uid: 0)
BUG: unable to handle kernel paging request at ffff8804615cfd78
...
RIP: ffff8804615cfd78
RSP: ffff8804615f0000
RBP: ffff8804615cfdc0
...
Call Trace:
do_signal+0x23
exit_to_usermode_loop+0x64
...
The RIP is on the same page as RBP, so it apparently started executing
on the stack.
The bug was bisected to commit ef0f3ed5a4ac ("x86/asm/power: Create
stack frames in hibernate_asm_64.S"), which in retrospect seems quite
dangerous, since that code saves and restores the stack pointer from a
global variable ('saved_context').
There are a lot of moving parts in the hibernate save and restore paths,
so I don't know exactly what caused the panic. Presumably, a FRAME_END
was executed without the corresponding FRAME_BEGIN, or vice versa. That
would corrupt the return address on the stack and would be consistent
with the details of the above panic.
Instead of doing the frame pointer save/restore around the bounds of the
affected functions, instead just do it around the call to swsusp_save().
That has the same effect of ensuring that if swsusp_save() sleeps, the
frame pointers will be correct. It's also a much more obviously safe
way to do it than the original patch. And objtool still doesn't report
any warnings.
Fixes: ef0f3ed5a4ac ("x86/asm/power: Create stack frames in hibernate_asm_64.S")
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=150021
Reported-by: <shuzzle@mailbox.org>
Tested-by: <shuzzle@mailbox.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
arch/x86/power/hibernate_asm_64.S | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/x86/power/hibernate_asm_64.S b/arch/x86/power/hibernate_asm_64.S
index 3177c2b..8eee0e9 100644
--- a/arch/x86/power/hibernate_asm_64.S
+++ b/arch/x86/power/hibernate_asm_64.S
@@ -24,7 +24,6 @@
#include <asm/frame.h>
ENTRY(swsusp_arch_suspend)
- FRAME_BEGIN
movq $saved_context, %rax
movq %rsp, pt_regs_sp(%rax)
movq %rbp, pt_regs_bp(%rax)
@@ -48,6 +47,7 @@ ENTRY(swsusp_arch_suspend)
movq %cr3, %rax
movq %rax, restore_cr3(%rip)
+ FRAME_BEGIN
call swsusp_save
FRAME_END
ret
@@ -104,7 +104,6 @@ ENTRY(core_restore_code)
/* code below belongs to the image kernel */
.align PAGE_SIZE
ENTRY(restore_registers)
- FRAME_BEGIN
/* go back to the original page tables */
movq %r9, %cr3
@@ -145,6 +144,5 @@ ENTRY(restore_registers)
/* tell the hibernation core that we've just restored the memory */
movq %rax, in_suspend(%rip)
- FRAME_END
ret
ENDPROC(restore_registers)
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-07-28 17:40 +0200 |
| Subject | Re: [PATCH] x86/asm/power: Fix hibernation return address corruption |
| Message-ID | <rZVzI-8cC-23@gated-at.bofh.it> |
| In reply to | #1451975 |
On Thu, Jul 28, 2016 at 10:17:07AM -0500, Josh Poimboeuf wrote:
> On Thu, Jul 28, 2016 at 01:29:49AM +0200, Rafael J. Wysocki wrote:
> > On Thursday, July 28, 2016 01:20:53 AM Rafael J. Wysocki wrote:
> > > On Wednesday, July 27, 2016 05:17:38 PM Josh Poimboeuf wrote:
> > > > On Thu, Jul 28, 2016 at 12:12:15AM +0200, Rafael J. Wysocki wrote:
> > > > > On Wednesday, July 27, 2016 12:59:18 PM Josh Poimboeuf wrote:
> > > > > > Hm... I have a theory, but I'm not sure about it. I noticed that
> > > > > > x86_acpi_enter_sleep_state(),
> > > > >
> > > > > I think you mean x86_acpi_suspend_lowlevel().
> > > >
> > > > Oops!
> > > >
> > > > > > which is involved in suspend, overwrites
> > > > > > several global variables (e.g, initial_code) which are used by the CPU
> > > > > > boot code in head_64.S. But surprisingly, it doesn't restore those
> > > > > > variables to their original values after it resumes.
> > > > >
> > > > > Is the head_64.S code also used to bring up offline CPUs?
> > > >
> > > > Yes.
> > >
> > > OK
> > >
> > > So it is really interesting why and how that stuff works for everybody.
> > >
> > > Basically, CPU online should fail after a suspend-resume cycle, but it
> > > doesn't most of the time AFAICS.
> >
> > do_boot_cpu() restores those values, so I think we're safe from that angle.
> >
> > That should apply to the CPU online during resume from hibernation too.
>
> Yeah, my theory was bogus. And as it turns out, the bug reporter made a
> mistake in the bisect. The actual offending commit was apparently:
>
> ef0f3ed5a4ac ("x86/asm/power: Create stack frames in hibernate_asm_64.S")
>
> Amazingly enough, I authored that patch as well. I think "git bisect"
> doesn't like me!
>
> Here's the fix:
>
> ----
>
> From: Josh Poimboeuf <jpoimboe@redhat.com>
> Subject: [PATCH] x86/asm/power: Fix hibernation return address corruption
>
> In kernel bug 150021, a kernel panic was reported when restoring a
> hibernate image. Only a picture of the oops was reported, so I can't
> paste the whole thing here. But here are the most interesting parts:
>
> kernel tried to execute NX-protected page - exploit attempt? (uid: 0)
> BUG: unable to handle kernel paging request at ffff8804615cfd78
> ...
> RIP: ffff8804615cfd78
> RSP: ffff8804615f0000
> RBP: ffff8804615cfdc0
> ...
> Call Trace:
> do_signal+0x23
> exit_to_usermode_loop+0x64
> ...
>
> The RIP is on the same page as RBP, so it apparently started executing
> on the stack.
>
> The bug was bisected to commit ef0f3ed5a4ac ("x86/asm/power: Create
> stack frames in hibernate_asm_64.S"), which in retrospect seems quite
> dangerous, since that code saves and restores the stack pointer from a
> global variable ('saved_context').
>
> There are a lot of moving parts in the hibernate save and restore paths,
> so I don't know exactly what caused the panic. Presumably, a FRAME_END
> was executed without the corresponding FRAME_BEGIN, or vice versa. That
> would corrupt the return address on the stack and would be consistent
> with the details of the above panic.
>
> Instead of doing the frame pointer save/restore around the bounds of the
> affected functions, instead just do it around the call to swsusp_save().
> That has the same effect of ensuring that if swsusp_save() sleeps, the
> frame pointers will be correct. It's also a much more obviously safe
> way to do it than the original patch. And objtool still doesn't report
> any warnings.
>
> Fixes: ef0f3ed5a4ac ("x86/asm/power: Create stack frames in hibernate_asm_64.S")
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=150021
> Reported-by: <shuzzle@mailbox.org>
> Tested-by: <shuzzle@mailbox.org>
Actually, Andre gave me his real name and email, so these should be:
Reported-by: Andre Reinke <andre.reinke@mailbox.org>
Tested-by: Andre Reinke <andre.reinke@mailbox.org>
--
Josh
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-07-28 23:40 +0200 |
| Subject | Re: [PATCH] x86/asm/power: Fix hibernation return address corruption |
| Message-ID | <s01c6-3wf-17@gated-at.bofh.it> |
| In reply to | #1451975 |
On Thursday, July 28, 2016 10:17:07 AM Josh Poimboeuf wrote:
> On Thu, Jul 28, 2016 at 01:29:49AM +0200, Rafael J. Wysocki wrote:
> > On Thursday, July 28, 2016 01:20:53 AM Rafael J. Wysocki wrote:
> > > On Wednesday, July 27, 2016 05:17:38 PM Josh Poimboeuf wrote:
> > > > On Thu, Jul 28, 2016 at 12:12:15AM +0200, Rafael J. Wysocki wrote:
> > > > > On Wednesday, July 27, 2016 12:59:18 PM Josh Poimboeuf wrote:
> > > > > > Hm... I have a theory, but I'm not sure about it. I noticed that
> > > > > > x86_acpi_enter_sleep_state(),
> > > > >
> > > > > I think you mean x86_acpi_suspend_lowlevel().
> > > >
> > > > Oops!
> > > >
> > > > > > which is involved in suspend, overwrites
> > > > > > several global variables (e.g, initial_code) which are used by the CPU
> > > > > > boot code in head_64.S. But surprisingly, it doesn't restore those
> > > > > > variables to their original values after it resumes.
> > > > >
> > > > > Is the head_64.S code also used to bring up offline CPUs?
> > > >
> > > > Yes.
> > >
> > > OK
> > >
> > > So it is really interesting why and how that stuff works for everybody.
> > >
> > > Basically, CPU online should fail after a suspend-resume cycle, but it
> > > doesn't most of the time AFAICS.
> >
> > do_boot_cpu() restores those values, so I think we're safe from that angle.
> >
> > That should apply to the CPU online during resume from hibernation too.
>
> Yeah, my theory was bogus. And as it turns out, the bug reporter made a
> mistake in the bisect. The actual offending commit was apparently:
>
> ef0f3ed5a4ac ("x86/asm/power: Create stack frames in hibernate_asm_64.S")
>
> Amazingly enough, I authored that patch as well. I think "git bisect"
> doesn't like me!
>
> Here's the fix:
>
> ----
>
> From: Josh Poimboeuf <jpoimboe@redhat.com>
> Subject: [PATCH] x86/asm/power: Fix hibernation return address corruption
>
> In kernel bug 150021, a kernel panic was reported when restoring a
> hibernate image. Only a picture of the oops was reported, so I can't
> paste the whole thing here. But here are the most interesting parts:
>
> kernel tried to execute NX-protected page - exploit attempt? (uid: 0)
> BUG: unable to handle kernel paging request at ffff8804615cfd78
> ...
> RIP: ffff8804615cfd78
> RSP: ffff8804615f0000
> RBP: ffff8804615cfdc0
> ...
> Call Trace:
> do_signal+0x23
> exit_to_usermode_loop+0x64
> ...
>
> The RIP is on the same page as RBP, so it apparently started executing
> on the stack.
>
> The bug was bisected to commit ef0f3ed5a4ac ("x86/asm/power: Create
> stack frames in hibernate_asm_64.S"), which in retrospect seems quite
> dangerous, since that code saves and restores the stack pointer from a
> global variable ('saved_context').
>
> There are a lot of moving parts in the hibernate save and restore paths,
> so I don't know exactly what caused the panic. Presumably, a FRAME_END
> was executed without the corresponding FRAME_BEGIN, or vice versa. That
> would corrupt the return address on the stack and would be consistent
> with the details of the above panic.
One problem that I can see immediately is that the stack pointer may not
be valid any more by the time the FRAME_BEGIN in restore_registers() is
executed. The memory it points to (which used to be a stack area of the
restore kernel) may have been overwritten by some image memory contents
from before hibernation and that page frame may now be used for whatever
different purpose it had been allocated for before hibernation. If that
happens, the FRAME_BEGIN will corrupt that memory.
Embarrassingly enough, I have looked at that piece of code for tens of
times recently, but somehow I've never translated that FRAME_BEGIN into
a push instruction. :-/
> Instead of doing the frame pointer save/restore around the bounds of the
> affected functions, instead just do it around the call to swsusp_save().
> That has the same effect of ensuring that if swsusp_save() sleeps, the
> frame pointers will be correct. It's also a much more obviously safe
> way to do it than the original patch. And objtool still doesn't report
> any warnings.
>
> Fixes: ef0f3ed5a4ac ("x86/asm/power: Create stack frames in hibernate_asm_64.S")
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=150021
> Reported-by: <shuzzle@mailbox.org>
> Tested-by: <shuzzle@mailbox.org>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
I've queued this up as an urgent fix. I hope there are no objections.
Thanks,
Rafael
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2016-07-29 09:20 +0200 |
| Subject | Re: [PATCH] x86/asm/power: Fix hibernation return address corruption |
| Message-ID | <s0afn-1tL-5@gated-at.bofh.it> |
| In reply to | #1452127 |
* Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > Fixes: ef0f3ed5a4ac ("x86/asm/power: Create stack frames in hibernate_asm_64.S")
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=150021
> > Reported-by: <shuzzle@mailbox.org>
> > Tested-by: <shuzzle@mailbox.org>
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
>
> I've queued this up as an urgent fix. I hope there are no objections.
Looks good to me too!
Acked-by: Ingo Molnar <mingo@kernel.org>
Thanks,
Ingo
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-07-28 00:20 +0200 |
| Message-ID | <rZFlg-5mX-21@gated-at.bofh.it> |
| In reply to | #1451538 |
On Thursday, July 28, 2016 12:12:15 AM Rafael J. Wysocki wrote: > On Wednesday, July 27, 2016 12:59:18 PM Josh Poimboeuf wrote: > > On Wed, Jul 27, 2016 at 01:08:21AM +0200, Rafael J. Wysocki wrote: > > > On Wed, Jul 27, 2016 at 12:42 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > > > On Tuesday, July 26, 2016 04:53:19 PM Josh Poimboeuf wrote: > > > >> On Tue, Jul 26, 2016 at 10:15:39PM +0200, Rafael J. Wysocki wrote: > > > >> > On Tuesday, July 26, 2016 09:39:05 AM Josh Poimboeuf 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 the bug is still there in 4.7 and it goes away after reverting the above > > > >> > commit. I guess I'll send a revert then. > > > >> > > > >> Hm, the code in wakeup_64.S seems quite magical, but I can't figure out > > > >> why this change causes a panic. Is it really causing the panic or is it > > > >> uncovering some other bug? > > > > > > > > It doesn't matter really. > > > > > > > > It surely interacts with something in a really odd way, but that only means > > > > that its impact goes far beyond what was expected when it was applied. Its > > > > changelog is inadequate as a result and so on. > > > > > > > >> Maybe we should hold off on reverting until we understand the issue. > > > > > > > > Which very well may take forever. > > > > > > > > And AFAICS this is a fix for a theoretical issue and it *reliably* triggers a > > > > very practical kernel panic for this particular reporter. I'd rather live > > > > with the theoretical issue unfixed to be honest. > > > > > > Well, actually, the best part is that do_suspend_lowlevel() is not > > > even called during hibernation or resume from it. It only is called > > > during suspend-to-RAM. > > > > > > Question now is how the change made by the commit in question can > > > affect hibernation which is an unrelated code path. We know for a > > > fact that it does affect it, but how? > > > > Hm... I have a theory, but I'm not sure about it. I noticed that > > x86_acpi_enter_sleep_state(), > > I think you mean x86_acpi_suspend_lowlevel(). > > > which is involved in suspend, overwrites > > several global variables (e.g, initial_code) which are used by the CPU > > boot code in head_64.S. But surprisingly, it doesn't restore those > > variables to their original values after it resumes. > > Is the head_64.S code also used to bring up offline CPUs? > > If not, then this is not the problem, because hibernation doesn't use it > for the boot CPU anyway. > > > So if a suspend and resume were done before the hibernate, those > > variables would presumably have suspend-centric values, and the first > > time a CPU is brought up during the hibernation restore operation, it > > would jump to wakeup_long64() (the suspend resume function) instead of > > start_secondary (which is the normal CPU boot function). > > > > So, if true, that would explain why my patch triggers a bug: > > wakeup_long64() always[*] jumps to .Lresume_point, which my patch > > affected. Because of the FRAME_END, it would pop an extra value off the > > stack. So when restore_processor_state() returns, it would return to > > whatever random address is on the stack after the real RIP. Which is > > consistent with the oops from the bug. It had a bad instruction > > pointer, which looked like a stack address. > > OK, so why doesn't it break resume from suspend to RAM? wakeup_long64 is > invoked by the CPU startup code then and doesn't the FRAME_END affect > that too? Ah, I see. wakeup_long64 will restore RSP from saved_rsp and that points to the right address already. OK Thanks, Rafael
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web