Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1398518 > unrolled thread
| Started by | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| First post | 2016-05-10 23:00 +0200 |
| Last post | 2016-05-12 02:10 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH] x86/arch_prctl/64: restore accidentally removed put_cpu in ARCH_SET_GS Andy Lutomirski <luto@amacapital.net> - 2016-05-10 23:00 +0200
Re: [PATCH] x86/arch_prctl/64: restore accidentally removed put_cpu in ARCH_SET_GS Mateusz Guzik <mguzik@redhat.com> - 2016-05-11 22:40 +0200
Re: [PATCH] x86/arch_prctl/64: restore accidentally removed put_cpu in ARCH_SET_GS Andy Lutomirski <luto@amacapital.net> - 2016-05-12 02:10 +0200
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-05-10 23:00 +0200 |
| Subject | Re: [PATCH] x86/arch_prctl/64: restore accidentally removed put_cpu in ARCH_SET_GS |
| Message-ID | <rxmV4-1JH-1@gated-at.bofh.it> |
On Tue, May 10, 2016 at 1:56 PM, Mateusz Guzik <mguzik@redhat.com> wrote: > This fixes 731e33e39a5b95ad770 "Remove FSBASE/GSBASE < 4G optimization" Indeed. How did that survive lockdep? Acked-by: Andy Lutomirski <luto@kernel.org> > > Signed-off-by: Mateusz Guzik <mguzik@redhat.com> > --- > arch/x86/kernel/process_64.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c > index 4285f6a..6b16c36 100644 > --- a/arch/x86/kernel/process_64.c > +++ b/arch/x86/kernel/process_64.c > @@ -541,6 +541,7 @@ long do_arch_prctl(struct task_struct *task, int code, unsigned long addr) > load_gs_index(0); > ret = wrmsrl_safe(MSR_KERNEL_GS_BASE, addr); > } > + put_cpu(); > break; > case ARCH_SET_FS: > /* Not strictly needed for fs, but do it for symmetry > -- > 1.8.3.1 > -- Andy Lutomirski AMA Capital Management, LLC
[toc] | [next] | [standalone]
| From | Mateusz Guzik <mguzik@redhat.com> |
|---|---|
| Date | 2016-05-11 22:40 +0200 |
| Message-ID | <rxJ5g-7qx-23@gated-at.bofh.it> |
| In reply to | #1398518 |
On Tue, May 10, 2016 at 01:58:24PM -0700, Andy Lutomirski wrote:
> On Tue, May 10, 2016 at 1:56 PM, Mateusz Guzik <mguzik@redhat.com> wrote:
> > This fixes 731e33e39a5b95ad770 "Remove FSBASE/GSBASE < 4G optimization"
>
> Indeed. How did that survive lockdep?
>
lockdep_sys_exit only checks actual locks.
In the common path after return from particular syscall interrupts get
blindly disabled (as opposed to checking first that they are enabled).
preemption count is not checked in the fast path at all and is checked
elsewhere as a side effect of calls to e.g. schedule().
How about a hack along these lines (note I don't claim this is
committable as it is, but it should work):
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index ec138e5..5887bc7 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -303,6 +303,24 @@ static void syscall_slow_exit_work(struct pt_regs *regs, u32 cached_flags)
tracehook_report_syscall_exit(regs, step);
}
+#ifdef CONFIG_PROVE_LOCKING
+/*
+ * Called after syscall handlers return.
+ */
+__visible void syscall_assert_exit(struct pt_regs *regs)
+{
+ if (in_atomic() || irqs_disabled()) {
+ printk(KERN_ERR "invalid state on exit from syscall %ld: "
+ "in_atomic(): %d, irqs_disabled(): %d, pid: %d, "
+ "name: %s\n", regs->orig_ax, in_atomic(),
+ irqs_disabled(), current->pid, current->comm);
+ }
+
+ if (irqs_disabled())
+ local_irq_enable();
+}
+#endif
+
/*
* Called with IRQs on and fully valid regs. Returns with IRQs off in a
* state such that we can immediately switch to user mode.
@@ -314,9 +332,7 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)
CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
- if (IS_ENABLED(CONFIG_PROVE_LOCKING) &&
- WARN(irqs_disabled(), "syscall %ld left IRQs disabled", regs->orig_ax))
- local_irq_enable();
+ syscall_assert_exit(regs);
/*
* First do one-time work. If these work items are enabled, we
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 9ee0da1..6c5cc23 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -210,6 +210,12 @@ entry_SYSCALL_64_fastpath:
movq %rax, RAX(%rsp)
1:
+#ifdef CONFIG_PROVE_LOCKING
+ /*
+ * We want to validate bunch of stuff, which will clobber registers.
+ */
+ jmp 2f
+#endif
/*
* If we get here, then we know that pt_regs is clean for SYSRET64.
* If we see that no exit work is required (which we are required
@@ -236,6 +242,7 @@ entry_SYSCALL_64_fastpath:
*/
TRACE_IRQS_ON
ENABLE_INTERRUPTS(CLBR_NONE)
+2:
SAVE_EXTRA_REGS
movq %rsp, %rdi
call syscall_return_slowpath /* returns with IRQs disabled */
--
Mateusz Guzik
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-05-12 02:10 +0200 |
| Message-ID | <rxMmt-2gq-1@gated-at.bofh.it> |
| In reply to | #1399499 |
On May 11, 2016 1:35 PM, "Mateusz Guzik" <mguzik@redhat.com> wrote:
>
> On Tue, May 10, 2016 at 01:58:24PM -0700, Andy Lutomirski wrote:
> > On Tue, May 10, 2016 at 1:56 PM, Mateusz Guzik <mguzik@redhat.com> wrote:
> > > This fixes 731e33e39a5b95ad770 "Remove FSBASE/GSBASE < 4G optimization"
> >
> > Indeed. How did that survive lockdep?
> >
>
> lockdep_sys_exit only checks actual locks.
>
> In the common path after return from particular syscall interrupts get
> blindly disabled (as opposed to checking first that they are enabled).
> preemption count is not checked in the fast path at all and is checked
> elsewhere as a side effect of calls to e.g. schedule().
>
> How about a hack along these lines (note I don't claim this is
> committable as it is, but it should work):
>
> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> index ec138e5..5887bc7 100644
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -303,6 +303,24 @@ static void syscall_slow_exit_work(struct pt_regs *regs, u32 cached_flags)
> tracehook_report_syscall_exit(regs, step);
> }
>
> +#ifdef CONFIG_PROVE_LOCKING
> +/*
> + * Called after syscall handlers return.
> + */
> +__visible void syscall_assert_exit(struct pt_regs *regs)
> +{
> + if (in_atomic() || irqs_disabled()) {
> + printk(KERN_ERR "invalid state on exit from syscall %ld: "
> + "in_atomic(): %d, irqs_disabled(): %d, pid: %d, "
> + "name: %s\n", regs->orig_ax, in_atomic(),
> + irqs_disabled(), current->pid, current->comm);
> + }
> +
> + if (irqs_disabled())
> + local_irq_enable();
> +}
> +#endif
> +
> /*
> * Called with IRQs on and fully valid regs. Returns with IRQs off in a
> * state such that we can immediately switch to user mode.
> @@ -314,9 +332,7 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)
>
> CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
>
> - if (IS_ENABLED(CONFIG_PROVE_LOCKING) &&
> - WARN(irqs_disabled(), "syscall %ld left IRQs disabled", regs->orig_ax))
> - local_irq_enable();
> + syscall_assert_exit(regs);
>
> /*
> * First do one-time work. If these work items are enabled, we
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index 9ee0da1..6c5cc23 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -210,6 +210,12 @@ entry_SYSCALL_64_fastpath:
> movq %rax, RAX(%rsp)
> 1:
>
> +#ifdef CONFIG_PROVE_LOCKING
> + /*
> + * We want to validate bunch of stuff, which will clobber registers.
> + */
> + jmp 2f
> +#endif
> /*
> * If we get here, then we know that pt_regs is clean for SYSRET64.
> * If we see that no exit work is required (which we are required
> @@ -236,6 +242,7 @@ entry_SYSCALL_64_fastpath:
> */
> TRACE_IRQS_ON
> ENABLE_INTERRUPTS(CLBR_NONE)
> +2:
> SAVE_EXTRA_REGS
> movq %rsp, %rdi
> call syscall_return_slowpath /* returns with IRQs disabled */
It would be nice to do this in a cross-arch way. Maybe we could
extend lockdep_sys_exit? Ingo, do you think that would be reasonable?
--Andy
>
> --
> Mateusz Guzik
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web