Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1491242 > unrolled thread
| Started by | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| First post | 2016-09-26 14:00 +0200 |
| Last post | 2016-10-06 04:10 +0200 |
| Articles | 5 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] x86/entry/64: Fix context tracking state warning when load_gs_index fails Wanpeng Li <kernellwp@gmail.com> - 2016-09-26 14:00 +0200
Re: [PATCH] x86/entry/64: Fix context tracking state warning when load_gs_index fails Wanpeng Li <kernellwp@gmail.com> - 2016-09-29 04:10 +0200
Re: [PATCH] x86/entry/64: Fix context tracking state warning when load_gs_index fails Andy Lutomirski <luto@amacapital.net> - 2016-09-29 23:10 +0200
Re: [PATCH] x86/entry/64: Fix context tracking state warning when load_gs_index fails Wanpeng Li <kernellwp@gmail.com> - 2016-09-30 02:50 +0200
Re: [PATCH] x86/entry/64: Fix context tracking state warning when load_gs_index fails Andy Lutomirski <luto@amacapital.net> - 2016-10-06 04:10 +0200
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2016-09-26 14:00 +0200 |
| Subject | [PATCH] x86/entry/64: Fix context tracking state warning when load_gs_index fails |
| Message-ID | <slCJH-3nN-17@gated-at.bofh.it> |
From: Wanpeng Li <wanpeng.li@hotmail.com> WARNING: CPU: 0 PID: 3331 at arch/x86/entry/common.c:45 enter_from_user_mode+0x32/0x50 CPU: 0 PID: 3331 Comm: ldt_gdt_64 Not tainted 4.8.0-rc7+ #13 Call Trace: dump_stack+0x99/0xd0 __warn+0xd1/0xf0 warn_slowpath_null+0x1d/0x20 enter_from_user_mode+0x32/0x50 error_entry+0x6d/0xc0 ? general_protection+0x12/0x30 ? native_load_gs_index+0xd/0x20 ? do_set_thread_area+0x19c/0x1f0 SyS_set_thread_area+0x24/0x30 do_int80_syscall_32+0x7c/0x220 entry_INT80_compat+0x38/0x50 This can be reproduced by running the GS testcase of ldt_gdt test unit in selftests. do_int80_syscall_32() will call enter_form_user_mode() to convert context tracking state from user state to kernel state. The load_gs_index can fail with user gsbase, gsbase will be fixed up and proceed if this happen. However, enter_from_user_mode() will be called again in the fixed up path though it is context tracking kernel state currently. This patch fix it by just fixing up gsbase and telling lockdep that IRQs are off once load_gs_index failed with user gsbase. Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Andy Lutomirski <luto@kernel.org> Cc: Borislav Petkov <bp@alien8.de> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> --- arch/x86/entry/entry_64.S | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S index d172c61..dc1ec23 100644 --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -1045,7 +1045,9 @@ ENTRY(error_entry) * gsbase and proceed. We'll fix up the exception and land in * .Lgs_change's error handler with kernel gsbase. */ - jmp .Lerror_entry_from_usermode_swapgs + SWAPGS + TRACE_IRQS_OFF + ret .Lbstep_iret: /* Fix truncated RIP */ -- 1.9.1
[toc] | [next] | [standalone]
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2016-09-29 04:10 +0200 |
| Subject | Re: [PATCH] x86/entry/64: Fix context tracking state warning when load_gs_index fails |
| Message-ID | <smyXn-6n3-3@gated-at.bofh.it> |
| In reply to | #1491242 |
Sorry for the ping, hope it can catch the upcoming merge window. :) 2016-09-26 19:49 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>: > From: Wanpeng Li <wanpeng.li@hotmail.com> > > WARNING: CPU: 0 PID: 3331 at arch/x86/entry/common.c:45 enter_from_user_mode+0x32/0x50 > CPU: 0 PID: 3331 Comm: ldt_gdt_64 Not tainted 4.8.0-rc7+ #13 > Call Trace: > dump_stack+0x99/0xd0 > __warn+0xd1/0xf0 > warn_slowpath_null+0x1d/0x20 > enter_from_user_mode+0x32/0x50 > error_entry+0x6d/0xc0 > ? general_protection+0x12/0x30 > ? native_load_gs_index+0xd/0x20 > ? do_set_thread_area+0x19c/0x1f0 > SyS_set_thread_area+0x24/0x30 > do_int80_syscall_32+0x7c/0x220 > entry_INT80_compat+0x38/0x50 > > This can be reproduced by running the GS testcase of ldt_gdt test unit in > selftests. > > do_int80_syscall_32() will call enter_form_user_mode() to convert context > tracking state from user state to kernel state. The load_gs_index can fail > with user gsbase, gsbase will be fixed up and proceed if this happen. > However, enter_from_user_mode() will be called again in the fixed up path > though it is context tracking kernel state currently. > > This patch fix it by just fixing up gsbase and telling lockdep that IRQs > are off once load_gs_index failed with user gsbase. > > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Andy Lutomirski <luto@kernel.org> > Cc: Borislav Petkov <bp@alien8.de> > Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> > --- > arch/x86/entry/entry_64.S | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S > index d172c61..dc1ec23 100644 > --- a/arch/x86/entry/entry_64.S > +++ b/arch/x86/entry/entry_64.S > @@ -1045,7 +1045,9 @@ ENTRY(error_entry) > * gsbase and proceed. We'll fix up the exception and land in > * .Lgs_change's error handler with kernel gsbase. > */ > - jmp .Lerror_entry_from_usermode_swapgs > + SWAPGS > + TRACE_IRQS_OFF > + ret > > .Lbstep_iret: > /* Fix truncated RIP */ > -- > 1.9.1 >
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-09-29 23:10 +0200 |
| Subject | Re: [PATCH] x86/entry/64: Fix context tracking state warning when load_gs_index fails |
| Message-ID | <smQKC-TX-55@gated-at.bofh.it> |
| In reply to | #1491242 |
On Mon, Sep 26, 2016 at 4:49 AM, Wanpeng Li <kernellwp@gmail.com> wrote: > From: Wanpeng Li <wanpeng.li@hotmail.com> > > WARNING: CPU: 0 PID: 3331 at arch/x86/entry/common.c:45 enter_from_user_mode+0x32/0x50 > CPU: 0 PID: 3331 Comm: ldt_gdt_64 Not tainted 4.8.0-rc7+ #13 > Call Trace: > dump_stack+0x99/0xd0 > __warn+0xd1/0xf0 > warn_slowpath_null+0x1d/0x20 > enter_from_user_mode+0x32/0x50 > error_entry+0x6d/0xc0 > ? general_protection+0x12/0x30 > ? native_load_gs_index+0xd/0x20 > ? do_set_thread_area+0x19c/0x1f0 > SyS_set_thread_area+0x24/0x30 > do_int80_syscall_32+0x7c/0x220 > entry_INT80_compat+0x38/0x50 > > This can be reproduced by running the GS testcase of ldt_gdt test unit in > selftests. > > do_int80_syscall_32() will call enter_form_user_mode() to convert context > tracking state from user state to kernel state. The load_gs_index can fail > with user gsbase, gsbase will be fixed up and proceed if this happen. > However, enter_from_user_mode() will be called again in the fixed up path > though it is context tracking kernel state currently. > > This patch fix it by just fixing up gsbase and telling lockdep that IRQs > are off once load_gs_index failed with user gsbase. > > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Andy Lutomirski <luto@kernel.org> > Cc: Borislav Petkov <bp@alien8.de> > Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> > --- > arch/x86/entry/entry_64.S | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S > index d172c61..dc1ec23 100644 > --- a/arch/x86/entry/entry_64.S > +++ b/arch/x86/entry/entry_64.S > @@ -1045,7 +1045,9 @@ ENTRY(error_entry) > * gsbase and proceed. We'll fix up the exception and land in > * .Lgs_change's error handler with kernel gsbase. > */ > - jmp .Lerror_entry_from_usermode_swapgs > + SWAPGS > + TRACE_IRQS_OFF Let's make this more readable: can you change this to: SWAPGS jmp .Lerror_entry_done and remove the .Lerror_entry_from_usermode_swapgs label as well? Thanks, Andy -- Andy Lutomirski AMA Capital Management, LLC
[toc] | [prev] | [next] | [standalone]
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2016-09-30 02:50 +0200 |
| Subject | Re: [PATCH] x86/entry/64: Fix context tracking state warning when load_gs_index fails |
| Message-ID | <smUbv-35g-5@gated-at.bofh.it> |
| In reply to | #1493726 |
2016-09-30 5:01 GMT+08:00 Andy Lutomirski <luto@amacapital.net>: > On Mon, Sep 26, 2016 at 4:49 AM, Wanpeng Li <kernellwp@gmail.com> wrote: >> From: Wanpeng Li <wanpeng.li@hotmail.com> >> >> WARNING: CPU: 0 PID: 3331 at arch/x86/entry/common.c:45 enter_from_user_mode+0x32/0x50 >> CPU: 0 PID: 3331 Comm: ldt_gdt_64 Not tainted 4.8.0-rc7+ #13 >> Call Trace: >> dump_stack+0x99/0xd0 >> __warn+0xd1/0xf0 >> warn_slowpath_null+0x1d/0x20 >> enter_from_user_mode+0x32/0x50 >> error_entry+0x6d/0xc0 >> ? general_protection+0x12/0x30 >> ? native_load_gs_index+0xd/0x20 >> ? do_set_thread_area+0x19c/0x1f0 >> SyS_set_thread_area+0x24/0x30 >> do_int80_syscall_32+0x7c/0x220 >> entry_INT80_compat+0x38/0x50 >> >> This can be reproduced by running the GS testcase of ldt_gdt test unit in >> selftests. >> >> do_int80_syscall_32() will call enter_form_user_mode() to convert context >> tracking state from user state to kernel state. The load_gs_index can fail >> with user gsbase, gsbase will be fixed up and proceed if this happen. >> However, enter_from_user_mode() will be called again in the fixed up path >> though it is context tracking kernel state currently. >> >> This patch fix it by just fixing up gsbase and telling lockdep that IRQs >> are off once load_gs_index failed with user gsbase. >> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: "H. Peter Anvin" <hpa@zytor.com> >> Cc: Andy Lutomirski <luto@kernel.org> >> Cc: Borislav Petkov <bp@alien8.de> >> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> >> --- >> arch/x86/entry/entry_64.S | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S >> index d172c61..dc1ec23 100644 >> --- a/arch/x86/entry/entry_64.S >> +++ b/arch/x86/entry/entry_64.S >> @@ -1045,7 +1045,9 @@ ENTRY(error_entry) >> * gsbase and proceed. We'll fix up the exception and land in >> * .Lgs_change's error handler with kernel gsbase. >> */ >> - jmp .Lerror_entry_from_usermode_swapgs >> + SWAPGS >> + TRACE_IRQS_OFF > > Let's make this more readable: can you change this to: > > SWAPGS > jmp .Lerror_entry_done > > and remove the .Lerror_entry_from_usermode_swapgs label as well? I will do this in v2. Btw, could you point out why the GS testcase in tools/testing/selftests/x86/ldt_gdt.c will #GP? SDM said that swapgs will #GP if CPL != 0, however, native_load_gs_index() is CPL == 0. Regards, Wanpeng Li
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-10-06 04:10 +0200 |
| Subject | Re: [PATCH] x86/entry/64: Fix context tracking state warning when load_gs_index fails |
| Message-ID | <sp6id-20i-7@gated-at.bofh.it> |
| In reply to | #1493805 |
On Sep 29, 2016 5:43 PM, "Wanpeng Li" <kernellwp@gmail.com> wrote: > > 2016-09-30 5:01 GMT+08:00 Andy Lutomirski <luto@amacapital.net>: > > On Mon, Sep 26, 2016 at 4:49 AM, Wanpeng Li <kernellwp@gmail.com> wrote: > >> From: Wanpeng Li <wanpeng.li@hotmail.com> > >> > >> WARNING: CPU: 0 PID: 3331 at arch/x86/entry/common.c:45 enter_from_user_mode+0x32/0x50 > >> CPU: 0 PID: 3331 Comm: ldt_gdt_64 Not tainted 4.8.0-rc7+ #13 > >> Call Trace: > >> dump_stack+0x99/0xd0 > >> __warn+0xd1/0xf0 > >> warn_slowpath_null+0x1d/0x20 > >> enter_from_user_mode+0x32/0x50 > >> error_entry+0x6d/0xc0 > >> ? general_protection+0x12/0x30 > >> ? native_load_gs_index+0xd/0x20 > >> ? do_set_thread_area+0x19c/0x1f0 > >> SyS_set_thread_area+0x24/0x30 > >> do_int80_syscall_32+0x7c/0x220 > >> entry_INT80_compat+0x38/0x50 > >> > >> This can be reproduced by running the GS testcase of ldt_gdt test unit in > >> selftests. > >> > >> do_int80_syscall_32() will call enter_form_user_mode() to convert context > >> tracking state from user state to kernel state. The load_gs_index can fail > >> with user gsbase, gsbase will be fixed up and proceed if this happen. > >> However, enter_from_user_mode() will be called again in the fixed up path > >> though it is context tracking kernel state currently. > >> > >> This patch fix it by just fixing up gsbase and telling lockdep that IRQs > >> are off once load_gs_index failed with user gsbase. > >> > >> Cc: Thomas Gleixner <tglx@linutronix.de> > >> Cc: Ingo Molnar <mingo@redhat.com> > >> Cc: "H. Peter Anvin" <hpa@zytor.com> > >> Cc: Andy Lutomirski <luto@kernel.org> > >> Cc: Borislav Petkov <bp@alien8.de> > >> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> > >> --- > >> arch/x86/entry/entry_64.S | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S > >> index d172c61..dc1ec23 100644 > >> --- a/arch/x86/entry/entry_64.S > >> +++ b/arch/x86/entry/entry_64.S > >> @@ -1045,7 +1045,9 @@ ENTRY(error_entry) > >> * gsbase and proceed. We'll fix up the exception and land in > >> * .Lgs_change's error handler with kernel gsbase. > >> */ > >> - jmp .Lerror_entry_from_usermode_swapgs > >> + SWAPGS > >> + TRACE_IRQS_OFF > > > > Let's make this more readable: can you change this to: > > > > SWAPGS > > jmp .Lerror_entry_done > > > > and remove the .Lerror_entry_from_usermode_swapgs label as well? > > I will do this in v2. Btw, could you point out why the GS testcase in > tools/testing/selftests/x86/ldt_gdt.c will #GP? SDM said that swapgs > will #GP if CPL != 0, however, native_load_gs_index() is CPL == 0. The fault is on MOV, not SWAPGS. --Andy
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web