* [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables @ 2016-08-11 21:49 ` Thomas Garnier 0 siblings, 0 replies; 18+ messages in thread From: Thomas Garnier @ 2016-08-11 21:49 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Pavel Machek Cc: linux-pm, linux-kernel, keescook, kernel-hardening, jikos, bpetkov, yinghai, Thomas Garnier Restore the processor state before calling any other function to ensure per-cpu variables can be used with KASLR memory randomization. Tracing functions use per-cpu variables (gs based) and one was called just before restoring the processor state fully. It resulted in a double fault when both the tracing & the exception handler functions tried to use a per-cpu variable. Signed-off-by: Thomas Garnier <thgarnie@google.com> --- Based on next-20160808 Thanks to Rafael, Jiri & Borislav in tracking down this bug. --- kernel/power/hibernate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index a881c6a..33c79b6 100644 --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -300,12 +300,12 @@ static int create_image(int platform_mode) save_processor_state(); trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, true); error = swsusp_arch_suspend(); + /* Restore control flow magically appears here */ + restore_processor_state(); trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, false); if (error) printk(KERN_ERR "PM: Error %d creating hibernation image\n", error); - /* Restore control flow magically appears here */ - restore_processor_state(); if (!in_suspend) events_check_enabled = false; -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [kernel-hardening] [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables @ 2016-08-11 21:49 ` Thomas Garnier 0 siblings, 0 replies; 18+ messages in thread From: Thomas Garnier @ 2016-08-11 21:49 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Pavel Machek Cc: linux-pm, linux-kernel, keescook, kernel-hardening, jikos, bpetkov, yinghai, Thomas Garnier Restore the processor state before calling any other function to ensure per-cpu variables can be used with KASLR memory randomization. Tracing functions use per-cpu variables (gs based) and one was called just before restoring the processor state fully. It resulted in a double fault when both the tracing & the exception handler functions tried to use a per-cpu variable. Signed-off-by: Thomas Garnier <thgarnie@google.com> --- Based on next-20160808 Thanks to Rafael, Jiri & Borislav in tracking down this bug. --- kernel/power/hibernate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index a881c6a..33c79b6 100644 --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -300,12 +300,12 @@ static int create_image(int platform_mode) save_processor_state(); trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, true); error = swsusp_arch_suspend(); + /* Restore control flow magically appears here */ + restore_processor_state(); trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, false); if (error) printk(KERN_ERR "PM: Error %d creating hibernation image\n", error); - /* Restore control flow magically appears here */ - restore_processor_state(); if (!in_suspend) events_check_enabled = false; -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables 2016-08-11 21:49 ` [kernel-hardening] " Thomas Garnier @ 2016-08-12 5:49 ` Borislav Petkov -1 siblings, 0 replies; 18+ messages in thread From: Borislav Petkov @ 2016-08-12 5:49 UTC (permalink / raw) To: Thomas Garnier, Rafael J . Wysocki Cc: Len Brown, Pavel Machek, linux-pm, linux-kernel, keescook, kernel-hardening, jikos, yinghai On Thu, Aug 11, 2016 at 02:49:29PM -0700, Thomas Garnier wrote: > Restore the processor state before calling any other function to ensure > per-cpu variables can be used with KASLR memory randomization. > > Tracing functions use per-cpu variables (gs based) and one was called > just before restoring the processor state fully. It resulted in a double > fault when both the tracing & the exception handler functions tried to > use a per-cpu variable. > > Signed-off-by: Thomas Garnier <thgarnie@google.com> > --- > Based on next-20160808 Ok, I believe before I test this, I need to apply another patch from Rafael. I think it is the "Always create temporary identity mapping correctly" thing. Yes, no? Rafael, can you please apply everything on a test branch for us to run? Thanks. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 18+ messages in thread
* [kernel-hardening] Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables @ 2016-08-12 5:49 ` Borislav Petkov 0 siblings, 0 replies; 18+ messages in thread From: Borislav Petkov @ 2016-08-12 5:49 UTC (permalink / raw) To: Thomas Garnier, Rafael J . Wysocki Cc: Len Brown, Pavel Machek, linux-pm, linux-kernel, keescook, kernel-hardening, jikos, yinghai On Thu, Aug 11, 2016 at 02:49:29PM -0700, Thomas Garnier wrote: > Restore the processor state before calling any other function to ensure > per-cpu variables can be used with KASLR memory randomization. > > Tracing functions use per-cpu variables (gs based) and one was called > just before restoring the processor state fully. It resulted in a double > fault when both the tracing & the exception handler functions tried to > use a per-cpu variable. > > Signed-off-by: Thomas Garnier <thgarnie@google.com> > --- > Based on next-20160808 Ok, I believe before I test this, I need to apply another patch from Rafael. I think it is the "Always create temporary identity mapping correctly" thing. Yes, no? Rafael, can you please apply everything on a test branch for us to run? Thanks. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables 2016-08-12 5:49 ` [kernel-hardening] " Borislav Petkov @ 2016-08-12 11:14 ` Rafael J. Wysocki -1 siblings, 0 replies; 18+ messages in thread From: Rafael J. Wysocki @ 2016-08-12 11:14 UTC (permalink / raw) To: Borislav Petkov Cc: Thomas Garnier, Rafael J . Wysocki, Len Brown, Pavel Machek, Linux PM, Linux Kernel Mailing List, Kees Cook, Kernel Hardening, Jiri Kosina, Yinghai Lu On Fri, Aug 12, 2016 at 7:49 AM, Borislav Petkov <bp@suse.de> wrote: > On Thu, Aug 11, 2016 at 02:49:29PM -0700, Thomas Garnier wrote: >> Restore the processor state before calling any other function to ensure >> per-cpu variables can be used with KASLR memory randomization. >> >> Tracing functions use per-cpu variables (gs based) and one was called >> just before restoring the processor state fully. It resulted in a double >> fault when both the tracing & the exception handler functions tried to >> use a per-cpu variable. >> >> Signed-off-by: Thomas Garnier <thgarnie@google.com> >> --- >> Based on next-20160808 > > Ok, I believe before I test this, I need to apply another patch from > Rafael. I think it is the "Always create temporary identity mapping > correctly" thing. > > Yes, no? Yes. > Rafael, can you please apply everything on a test branch for us to run? You can simply test my linux-next branch: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \ linux-next That's 4.8-rc1 plus 3 fixes on top of it. Thanks, Rafael ^ permalink raw reply [flat|nested] 18+ messages in thread
* [kernel-hardening] Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables @ 2016-08-12 11:14 ` Rafael J. Wysocki 0 siblings, 0 replies; 18+ messages in thread From: Rafael J. Wysocki @ 2016-08-12 11:14 UTC (permalink / raw) To: Borislav Petkov Cc: Thomas Garnier, Rafael J . Wysocki, Len Brown, Pavel Machek, Linux PM, Linux Kernel Mailing List, Kees Cook, Kernel Hardening, Jiri Kosina, Yinghai Lu On Fri, Aug 12, 2016 at 7:49 AM, Borislav Petkov <bp@suse.de> wrote: > On Thu, Aug 11, 2016 at 02:49:29PM -0700, Thomas Garnier wrote: >> Restore the processor state before calling any other function to ensure >> per-cpu variables can be used with KASLR memory randomization. >> >> Tracing functions use per-cpu variables (gs based) and one was called >> just before restoring the processor state fully. It resulted in a double >> fault when both the tracing & the exception handler functions tried to >> use a per-cpu variable. >> >> Signed-off-by: Thomas Garnier <thgarnie@google.com> >> --- >> Based on next-20160808 > > Ok, I believe before I test this, I need to apply another patch from > Rafael. I think it is the "Always create temporary identity mapping > correctly" thing. > > Yes, no? Yes. > Rafael, can you please apply everything on a test branch for us to run? You can simply test my linux-next branch: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \ linux-next That's 4.8-rc1 plus 3 fixes on top of it. Thanks, Rafael ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables 2016-08-12 11:14 ` [kernel-hardening] " Rafael J. Wysocki @ 2016-08-12 16:03 ` Thomas Garnier -1 siblings, 0 replies; 18+ messages in thread From: Thomas Garnier @ 2016-08-12 16:03 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Borislav Petkov, Rafael J . Wysocki, Len Brown, Pavel Machek, Linux PM, Linux Kernel Mailing List, Kees Cook, Kernel Hardening, Jiri Kosina, Yinghai Lu On Fri, Aug 12, 2016 at 4:14 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Fri, Aug 12, 2016 at 7:49 AM, Borislav Petkov <bp@suse.de> wrote: >> On Thu, Aug 11, 2016 at 02:49:29PM -0700, Thomas Garnier wrote: >>> Restore the processor state before calling any other function to ensure >>> per-cpu variables can be used with KASLR memory randomization. >>> >>> Tracing functions use per-cpu variables (gs based) and one was called >>> just before restoring the processor state fully. It resulted in a double >>> fault when both the tracing & the exception handler functions tried to >>> use a per-cpu variable. >>> >>> Signed-off-by: Thomas Garnier <thgarnie@google.com> >>> --- >>> Based on next-20160808 >> >> Ok, I believe before I test this, I need to apply another patch from >> Rafael. I think it is the "Always create temporary identity mapping >> correctly" thing. >> >> Yes, no? > > Yes. > >> Rafael, can you please apply everything on a test branch for us to run? > > You can simply test my linux-next branch: > > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \ > linux-next > > That's 4.8-rc1 plus 3 fixes on top of it. Borislav, let me know once you tested it and I will send a v2 with acked/tested. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [kernel-hardening] Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables @ 2016-08-12 16:03 ` Thomas Garnier 0 siblings, 0 replies; 18+ messages in thread From: Thomas Garnier @ 2016-08-12 16:03 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Borislav Petkov, Rafael J . Wysocki, Len Brown, Pavel Machek, Linux PM, Linux Kernel Mailing List, Kees Cook, Kernel Hardening, Jiri Kosina, Yinghai Lu On Fri, Aug 12, 2016 at 4:14 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Fri, Aug 12, 2016 at 7:49 AM, Borislav Petkov <bp@suse.de> wrote: >> On Thu, Aug 11, 2016 at 02:49:29PM -0700, Thomas Garnier wrote: >>> Restore the processor state before calling any other function to ensure >>> per-cpu variables can be used with KASLR memory randomization. >>> >>> Tracing functions use per-cpu variables (gs based) and one was called >>> just before restoring the processor state fully. It resulted in a double >>> fault when both the tracing & the exception handler functions tried to >>> use a per-cpu variable. >>> >>> Signed-off-by: Thomas Garnier <thgarnie@google.com> >>> --- >>> Based on next-20160808 >> >> Ok, I believe before I test this, I need to apply another patch from >> Rafael. I think it is the "Always create temporary identity mapping >> correctly" thing. >> >> Yes, no? > > Yes. > >> Rafael, can you please apply everything on a test branch for us to run? > > You can simply test my linux-next branch: > > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \ > linux-next > > That's 4.8-rc1 plus 3 fixes on top of it. Borislav, let me know once you tested it and I will send a v2 with acked/tested. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables 2016-08-12 16:03 ` [kernel-hardening] " Thomas Garnier @ 2016-08-12 17:45 ` Borislav Petkov -1 siblings, 0 replies; 18+ messages in thread From: Borislav Petkov @ 2016-08-12 17:45 UTC (permalink / raw) To: Thomas Garnier Cc: Rafael J. Wysocki, Rafael J . Wysocki, Len Brown, Pavel Machek, Linux PM, Linux Kernel Mailing List, Kees Cook, Kernel Hardening, Jiri Kosina, Yinghai Lu On Fri, Aug 12, 2016 at 09:03:52AM -0700, Thomas Garnier wrote: > Borislav, let me know once you tested it and I will send a v2 with > acked/tested. Just did 5 s2d runs, back-to-back, with Rafael's linux-next branch. Looks good so far, no hickups. I'll watch it the coming days if there are any changes. Reported-and-tested-by: Borislav Petkov <bp@suse.de> Btw, I'm running with: CONFIG_ARCH_HAS_ELF_RANDOMIZE=y CONFIG_RANDOMIZE_BASE=y CONFIG_RANDOMIZE_MEMORY=y CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING=0x0 Thanks guys for the fixes. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 18+ messages in thread
* [kernel-hardening] Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables @ 2016-08-12 17:45 ` Borislav Petkov 0 siblings, 0 replies; 18+ messages in thread From: Borislav Petkov @ 2016-08-12 17:45 UTC (permalink / raw) To: Thomas Garnier Cc: Rafael J. Wysocki, Rafael J . Wysocki, Len Brown, Pavel Machek, Linux PM, Linux Kernel Mailing List, Kees Cook, Kernel Hardening, Jiri Kosina, Yinghai Lu On Fri, Aug 12, 2016 at 09:03:52AM -0700, Thomas Garnier wrote: > Borislav, let me know once you tested it and I will send a v2 with > acked/tested. Just did 5 s2d runs, back-to-back, with Rafael's linux-next branch. Looks good so far, no hickups. I'll watch it the coming days if there are any changes. Reported-and-tested-by: Borislav Petkov <bp@suse.de> Btw, I'm running with: CONFIG_ARCH_HAS_ELF_RANDOMIZE=y CONFIG_RANDOMIZE_BASE=y CONFIG_RANDOMIZE_MEMORY=y CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING=0x0 Thanks guys for the fixes. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables 2016-08-11 21:49 ` [kernel-hardening] " Thomas Garnier @ 2016-08-12 6:01 ` Jiri Kosina -1 siblings, 0 replies; 18+ messages in thread From: Jiri Kosina @ 2016-08-12 6:01 UTC (permalink / raw) To: Thomas Garnier Cc: Rafael J . Wysocki, Len Brown, Pavel Machek, linux-pm, linux-kernel, keescook, kernel-hardening, bpetkov, yinghai On Thu, 11 Aug 2016, Thomas Garnier wrote: > Restore the processor state before calling any other function to ensure > per-cpu variables can be used with KASLR memory randomization. > > Tracing functions use per-cpu variables (gs based) and one was called > just before restoring the processor state fully. It resulted in a double > fault when both the tracing & the exception handler functions tried to > use a per-cpu variable. > > Signed-off-by: Thomas Garnier <thgarnie@google.com> Alright, this did the trick, thanks :) Feel free to add Reported-by: Jiri Kosina <jkosina@suse.cz> Tested-by: Jiri Kosina <jkosina@suse.cz> One thing is still beyond me though ... how the heck this doesn't happen without DEBUG_LOCK_ALLOC? The percpu area pointer should be corrupted nevertheless, shouldn't it? Thanks, -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 18+ messages in thread
* [kernel-hardening] Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables @ 2016-08-12 6:01 ` Jiri Kosina 0 siblings, 0 replies; 18+ messages in thread From: Jiri Kosina @ 2016-08-12 6:01 UTC (permalink / raw) To: Thomas Garnier Cc: Rafael J . Wysocki, Len Brown, Pavel Machek, linux-pm, linux-kernel, keescook, kernel-hardening, bpetkov, yinghai On Thu, 11 Aug 2016, Thomas Garnier wrote: > Restore the processor state before calling any other function to ensure > per-cpu variables can be used with KASLR memory randomization. > > Tracing functions use per-cpu variables (gs based) and one was called > just before restoring the processor state fully. It resulted in a double > fault when both the tracing & the exception handler functions tried to > use a per-cpu variable. > > Signed-off-by: Thomas Garnier <thgarnie@google.com> Alright, this did the trick, thanks :) Feel free to add Reported-by: Jiri Kosina <jkosina@suse.cz> Tested-by: Jiri Kosina <jkosina@suse.cz> One thing is still beyond me though ... how the heck this doesn't happen without DEBUG_LOCK_ALLOC? The percpu area pointer should be corrupted nevertheless, shouldn't it? Thanks, -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables 2016-08-12 6:01 ` [kernel-hardening] " Jiri Kosina @ 2016-08-12 9:23 ` Jiri Kosina -1 siblings, 0 replies; 18+ messages in thread From: Jiri Kosina @ 2016-08-12 9:23 UTC (permalink / raw) To: Thomas Garnier Cc: Rafael J . Wysocki, Len Brown, Pavel Machek, linux-pm, linux-kernel, keescook, kernel-hardening, bpetkov, yinghai On Fri, 12 Aug 2016, Jiri Kosina wrote: > One thing is still beyond me though ... how the heck this doesn't happen > without DEBUG_LOCK_ALLOC? The percpu area pointer should be corrupted > nevertheless, shouldn't it? The reason is that turning DEBUG_LOCK_ALLOC changing trace_suspend_resume() from ffffffff810c7280 <trace_suspend_resume>: ffffffff810c7280: 55 push %rbp ffffffff810c7281: 48 89 e5 mov %rsp,%rbp ffffffff810c7284: 41 56 push %r14 ffffffff810c7286: 41 55 push %r13 ffffffff810c7288: 41 54 push %r12 ffffffff810c728a: 53 push %rbx ffffffff810c728b: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) ffffffff810c7290: 65 8b 05 59 2f f4 7e mov %gs:0x7ef42f59(%rip),%eax # a1f0 <cpu_number> ffffffff810c7297: 89 c0 mov %eax,%eax ffffffff810c7299: 48 0f a3 05 9f 2f c4 bt %rax,0xc42f9f(%rip) # ffffffff81d0a240 <__cpu_online_mask> to ffffffff810ad150 <trace_suspend_resume>: ffffffff810ad150: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) ffffffff810ad155: c3 retq ffffffff810ad156: 65 8b 05 93 d0 f5 7e mov %gs:0x7ef5d093(%rip),%eax # a1f0 <cpu_number> ffffffff810ad15d: 89 c0 mov %eax,%eax ffffffff810ad15f: 48 0f a3 05 59 0b c4 bt %rax,0xc40b59(%rip) # ffffffff81cedcc0 <__cpu_online_mask> ffffffff810ad166: 00 IOW, with the config change, trace_suspend_resume() returns immediately without trying to get the current SMP id. And it's because of DEBUG_LOCK_ALLOC implies LOCKDEP, and that does this to __DECLARE_TRACE() * When lockdep is enabled, we make sure to always do the RCU portions of * the tracepoint code, regardless of whether tracing is on. However, * don't check if the condition is false, due to interaction with idle * instrumentation. This lets us find RCU issues triggered with tracepoints * even when this tracepoint is off. This code has no purpose other than * poking RCU a bit. */ #define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \ extern struct tracepoint __tracepoint_##name; \ static inline void trace_##name(proto) \ { \ if (static_key_false(&__tracepoint_##name.key)) \ __DO_TRACE(&__tracepoint_##name, \ TP_PROTO(data_proto), \ TP_ARGS(data_args), \ TP_CONDITION(cond),,); \ if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) { \ rcu_read_lock_sched_notrace(); \ rcu_dereference_sched(__tracepoint_##name.funcs);\ rcu_read_unlock_sched_notrace(); \ } \ } That's pretty nasty, as turning LOCKDEP on has sideffects on the code that'd normally not be expected to be run at all (tracepoint off). Oh well. Thanks again, -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 18+ messages in thread
* [kernel-hardening] Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables @ 2016-08-12 9:23 ` Jiri Kosina 0 siblings, 0 replies; 18+ messages in thread From: Jiri Kosina @ 2016-08-12 9:23 UTC (permalink / raw) To: Thomas Garnier Cc: Rafael J . Wysocki, Len Brown, Pavel Machek, linux-pm, linux-kernel, keescook, kernel-hardening, bpetkov, yinghai On Fri, 12 Aug 2016, Jiri Kosina wrote: > One thing is still beyond me though ... how the heck this doesn't happen > without DEBUG_LOCK_ALLOC? The percpu area pointer should be corrupted > nevertheless, shouldn't it? The reason is that turning DEBUG_LOCK_ALLOC changing trace_suspend_resume() from ffffffff810c7280 <trace_suspend_resume>: ffffffff810c7280: 55 push %rbp ffffffff810c7281: 48 89 e5 mov %rsp,%rbp ffffffff810c7284: 41 56 push %r14 ffffffff810c7286: 41 55 push %r13 ffffffff810c7288: 41 54 push %r12 ffffffff810c728a: 53 push %rbx ffffffff810c728b: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) ffffffff810c7290: 65 8b 05 59 2f f4 7e mov %gs:0x7ef42f59(%rip),%eax # a1f0 <cpu_number> ffffffff810c7297: 89 c0 mov %eax,%eax ffffffff810c7299: 48 0f a3 05 9f 2f c4 bt %rax,0xc42f9f(%rip) # ffffffff81d0a240 <__cpu_online_mask> to ffffffff810ad150 <trace_suspend_resume>: ffffffff810ad150: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) ffffffff810ad155: c3 retq ffffffff810ad156: 65 8b 05 93 d0 f5 7e mov %gs:0x7ef5d093(%rip),%eax # a1f0 <cpu_number> ffffffff810ad15d: 89 c0 mov %eax,%eax ffffffff810ad15f: 48 0f a3 05 59 0b c4 bt %rax,0xc40b59(%rip) # ffffffff81cedcc0 <__cpu_online_mask> ffffffff810ad166: 00 IOW, with the config change, trace_suspend_resume() returns immediately without trying to get the current SMP id. And it's because of DEBUG_LOCK_ALLOC implies LOCKDEP, and that does this to __DECLARE_TRACE() * When lockdep is enabled, we make sure to always do the RCU portions of * the tracepoint code, regardless of whether tracing is on. However, * don't check if the condition is false, due to interaction with idle * instrumentation. This lets us find RCU issues triggered with tracepoints * even when this tracepoint is off. This code has no purpose other than * poking RCU a bit. */ #define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \ extern struct tracepoint __tracepoint_##name; \ static inline void trace_##name(proto) \ { \ if (static_key_false(&__tracepoint_##name.key)) \ __DO_TRACE(&__tracepoint_##name, \ TP_PROTO(data_proto), \ TP_ARGS(data_args), \ TP_CONDITION(cond),,); \ if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) { \ rcu_read_lock_sched_notrace(); \ rcu_dereference_sched(__tracepoint_##name.funcs);\ rcu_read_unlock_sched_notrace(); \ } \ } That's pretty nasty, as turning LOCKDEP on has sideffects on the code that'd normally not be expected to be run at all (tracepoint off). Oh well. Thanks again, -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables 2016-08-12 9:23 ` [kernel-hardening] " Jiri Kosina @ 2016-08-12 16:03 ` Thomas Garnier -1 siblings, 0 replies; 18+ messages in thread From: Thomas Garnier @ 2016-08-12 16:03 UTC (permalink / raw) To: Jiri Kosina Cc: Rafael J . Wysocki, Len Brown, Pavel Machek, Linux PM list, LKML, Kees Cook, Kernel Hardening, Borislav Petkov, Yinghai Lu On Fri, Aug 12, 2016 at 2:23 AM, Jiri Kosina <jikos@kernel.org> wrote: > On Fri, 12 Aug 2016, Jiri Kosina wrote: > > That's pretty nasty, as turning LOCKDEP on has sideffects on the code > that'd normally not be expected to be run at all (tracepoint off). > > Oh well. Thanks for the analysis, I didn't got that far so I had no idea how everything was connected. > Thanks again, Thanks you and Borislav for finding it. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [kernel-hardening] Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables @ 2016-08-12 16:03 ` Thomas Garnier 0 siblings, 0 replies; 18+ messages in thread From: Thomas Garnier @ 2016-08-12 16:03 UTC (permalink / raw) To: Jiri Kosina Cc: Rafael J . Wysocki, Len Brown, Pavel Machek, Linux PM list, LKML, Kees Cook, Kernel Hardening, Borislav Petkov, Yinghai Lu On Fri, Aug 12, 2016 at 2:23 AM, Jiri Kosina <jikos@kernel.org> wrote: > On Fri, 12 Aug 2016, Jiri Kosina wrote: > > That's pretty nasty, as turning LOCKDEP on has sideffects on the code > that'd normally not be expected to be run at all (tracepoint off). > > Oh well. Thanks for the analysis, I didn't got that far so I had no idea how everything was connected. > Thanks again, Thanks you and Borislav for finding it. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables 2016-08-11 21:49 ` [kernel-hardening] " Thomas Garnier @ 2016-08-12 6:29 ` Pavel Machek -1 siblings, 0 replies; 18+ messages in thread From: Pavel Machek @ 2016-08-12 6:29 UTC (permalink / raw) To: Thomas Garnier Cc: Rafael J . Wysocki, Len Brown, linux-pm, linux-kernel, keescook, kernel-hardening, jikos, bpetkov, yinghai Hi! > Restore the processor state before calling any other function to ensure > per-cpu variables can be used with KASLR memory randomization. > > Tracing functions use per-cpu variables (gs based) and one was called > just before restoring the processor state fully. It resulted in a double > fault when both the tracing & the exception handler functions tried to > use a per-cpu variable. > > Signed-off-by: Thomas Garnier <thgarnie@google.com> > --- > Based on next-20160808 > > Thanks to Rafael, Jiri & Borislav in tracking down this bug. > --- > kernel/power/hibernate.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c > index a881c6a..33c79b6 100644 > --- a/kernel/power/hibernate.c > +++ b/kernel/power/hibernate.c > @@ -300,12 +300,12 @@ static int create_image(int platform_mode) > save_processor_state(); > trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, true); > error = swsusp_arch_suspend(); > + /* Restore control flow magically appears here */ > + restore_processor_state(); > trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, false); > if (error) > printk(KERN_ERR "PM: Error %d creating hibernation image\n", > error); > - /* Restore control flow magically appears here */ > - restore_processor_state(); > if (!in_suspend) > events_check_enabled = false; > Ugh. Plus it also fixes very confusing situation where /* Restore control flow magically appears here */ comment was 4 lines away from where it _actually_ magically appeared. Good catch. Acked-by: Pavel Machek <pavel@ucw.cz> -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 18+ messages in thread
* [kernel-hardening] Re: [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables @ 2016-08-12 6:29 ` Pavel Machek 0 siblings, 0 replies; 18+ messages in thread From: Pavel Machek @ 2016-08-12 6:29 UTC (permalink / raw) To: Thomas Garnier Cc: Rafael J . Wysocki, Len Brown, linux-pm, linux-kernel, keescook, kernel-hardening, jikos, bpetkov, yinghai Hi! > Restore the processor state before calling any other function to ensure > per-cpu variables can be used with KASLR memory randomization. > > Tracing functions use per-cpu variables (gs based) and one was called > just before restoring the processor state fully. It resulted in a double > fault when both the tracing & the exception handler functions tried to > use a per-cpu variable. > > Signed-off-by: Thomas Garnier <thgarnie@google.com> > --- > Based on next-20160808 > > Thanks to Rafael, Jiri & Borislav in tracking down this bug. > --- > kernel/power/hibernate.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c > index a881c6a..33c79b6 100644 > --- a/kernel/power/hibernate.c > +++ b/kernel/power/hibernate.c > @@ -300,12 +300,12 @@ static int create_image(int platform_mode) > save_processor_state(); > trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, true); > error = swsusp_arch_suspend(); > + /* Restore control flow magically appears here */ > + restore_processor_state(); > trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, false); > if (error) > printk(KERN_ERR "PM: Error %d creating hibernation image\n", > error); > - /* Restore control flow magically appears here */ > - restore_processor_state(); > if (!in_suspend) > events_check_enabled = false; > Ugh. Plus it also fixes very confusing situation where /* Restore control flow magically appears here */ comment was 4 lines away from where it _actually_ magically appeared. Good catch. Acked-by: Pavel Machek <pavel@ucw.cz> -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-08-12 17:45 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-11 21:49 [PATCH v1] x86/power/64: Restore processor state before using per-cpu variables Thomas Garnier 2016-08-11 21:49 ` [kernel-hardening] " Thomas Garnier 2016-08-12 5:49 ` Borislav Petkov 2016-08-12 5:49 ` [kernel-hardening] " Borislav Petkov 2016-08-12 11:14 ` Rafael J. Wysocki 2016-08-12 11:14 ` [kernel-hardening] " Rafael J. Wysocki 2016-08-12 16:03 ` Thomas Garnier 2016-08-12 16:03 ` [kernel-hardening] " Thomas Garnier 2016-08-12 17:45 ` Borislav Petkov 2016-08-12 17:45 ` [kernel-hardening] " Borislav Petkov 2016-08-12 6:01 ` Jiri Kosina 2016-08-12 6:01 ` [kernel-hardening] " Jiri Kosina 2016-08-12 9:23 ` Jiri Kosina 2016-08-12 9:23 ` [kernel-hardening] " Jiri Kosina 2016-08-12 16:03 ` Thomas Garnier 2016-08-12 16:03 ` [kernel-hardening] " Thomas Garnier 2016-08-12 6:29 ` Pavel Machek 2016-08-12 6:29 ` [kernel-hardening] " Pavel Machek
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.