* [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
[not found] <cover.1593191971.git.luto@kernel.org>
@ 2020-06-26 17:21 ` Andy Lutomirski
2020-06-28 2:47 ` Boris Ostrovsky
2020-07-01 15:42 ` Brian Gerst
0 siblings, 2 replies; 5+ messages in thread
From: Andy Lutomirski @ 2020-06-26 17:21 UTC (permalink / raw)
To: x86
Cc: Juergen Gross, Stefano Stabellini, Andrew Cooper, linux-kernel,
Andy Lutomirski, xen-devel, Boris Ostrovsky
The SYSENTER frame setup was nonsense. It worked by accident
because the normal code into which the Xen asm jumped
(entry_SYSENTER_32/compat) threw away SP without touching the stack.
entry_SYSENTER_compat was recently modified such that it relied on
having a valid stack pointer, so now the Xen asm needs to invoke it
with a valid stack.
Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
metal prologue.
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org
Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/entry/entry_64_compat.S | 1 +
arch/x86/xen/xen-asm_64.S | 20 ++++++++++++++++----
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 7b9d8150f652..381a6de7de9c 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -79,6 +79,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
pushfq /* pt_regs->flags (except IF = 0) */
pushq $__USER32_CS /* pt_regs->cs */
pushq $0 /* pt_regs->ip = 0 (placeholder) */
+SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
pushq %rax /* pt_regs->orig_ax */
pushq %rdi /* pt_regs->di */
pushq %rsi /* pt_regs->si */
diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
index 5d252aaeade8..e1e1c7eafa60 100644
--- a/arch/x86/xen/xen-asm_64.S
+++ b/arch/x86/xen/xen-asm_64.S
@@ -161,10 +161,22 @@ SYM_FUNC_END(xen_syscall32_target)
/* 32-bit compat sysenter target */
SYM_FUNC_START(xen_sysenter_target)
- mov 0*8(%rsp), %rcx
- mov 1*8(%rsp), %r11
- mov 5*8(%rsp), %rsp
- jmp entry_SYSENTER_compat
+ /*
+ * NB: Xen is polite and clears TF from EFLAGS for us. This means
+ * that we don't need to guard against single step exceptions here.
+ */
+ popq %rcx
+ popq %r11
+
+ /*
+ * Neither Xen nor the kernel really knows what the old SS and
+ * CS were. The kernel expects __USER32_DS and __USER32_CS, so
+ * report those values even though Xen will guess its own values.
+ */
+ movq $__USER32_DS, 4*8(%rsp)
+ movq $__USER32_CS, 1*8(%rsp)
+
+ jmp entry_SYSENTER_compat_after_hwframe
SYM_FUNC_END(xen_sysenter_target)
#else /* !CONFIG_IA32_EMULATION */
--
2.25.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
2020-06-26 17:21 ` [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup Andy Lutomirski
@ 2020-06-28 2:47 ` Boris Ostrovsky
2020-07-01 15:42 ` Brian Gerst
1 sibling, 0 replies; 5+ messages in thread
From: Boris Ostrovsky @ 2020-06-28 2:47 UTC (permalink / raw)
To: Andy Lutomirski, x86
Cc: Juergen Gross, Andrew Cooper, Stefano Stabellini, linux-kernel,
xen-devel
On 6/26/20 1:21 PM, Andy Lutomirski wrote:
> The SYSENTER frame setup was nonsense. It worked by accident
> because the normal code into which the Xen asm jumped
> (entry_SYSENTER_32/compat) threw away SP without touching the stack.
> entry_SYSENTER_compat was recently modified such that it relied on
> having a valid stack pointer, so now the Xen asm needs to invoke it
> with a valid stack.
>
> Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
> metal prologue.
>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: xen-devel@lists.xenproject.org
> Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
2020-06-26 17:21 ` [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup Andy Lutomirski
2020-06-28 2:47 ` Boris Ostrovsky
@ 2020-07-01 15:42 ` Brian Gerst
2020-07-01 18:39 ` Andy Lutomirski
1 sibling, 1 reply; 5+ messages in thread
From: Brian Gerst @ 2020-07-01 15:42 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Juergen Gross, Stefano Stabellini, Andrew Cooper,
the arch/x86 maintainers, Linux Kernel Mailing List, xen-devel,
Boris Ostrovsky
On Fri, Jun 26, 2020 at 1:30 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> The SYSENTER frame setup was nonsense. It worked by accident
> because the normal code into which the Xen asm jumped
> (entry_SYSENTER_32/compat) threw away SP without touching the stack.
> entry_SYSENTER_compat was recently modified such that it relied on
> having a valid stack pointer, so now the Xen asm needs to invoke it
> with a valid stack.
>
> Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
> metal prologue.
>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: xen-devel@lists.xenproject.org
> Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
> arch/x86/entry/entry_64_compat.S | 1 +
> arch/x86/xen/xen-asm_64.S | 20 ++++++++++++++++----
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
> index 7b9d8150f652..381a6de7de9c 100644
> --- a/arch/x86/entry/entry_64_compat.S
> +++ b/arch/x86/entry/entry_64_compat.S
> @@ -79,6 +79,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
> pushfq /* pt_regs->flags (except IF = 0) */
> pushq $__USER32_CS /* pt_regs->cs */
> pushq $0 /* pt_regs->ip = 0 (placeholder) */
> +SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
This skips over the section that truncates the syscall number to
32-bits. The comments present some doubt that it is actually
necessary, but the Xen path shouldn't differ from native. That code
should be moved after this new label.
--
Brian Gerst
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
2020-07-01 15:42 ` Brian Gerst
@ 2020-07-01 18:39 ` Andy Lutomirski
2020-07-02 12:54 ` Thomas Gleixner
0 siblings, 1 reply; 5+ messages in thread
From: Andy Lutomirski @ 2020-07-01 18:39 UTC (permalink / raw)
To: Brian Gerst
Cc: Juergen Gross, Stefano Stabellini, Andrew Cooper,
the arch/x86 maintainers, Linux Kernel Mailing List,
Andy Lutomirski, xen-devel, Boris Ostrovsky
On Wed, Jul 1, 2020 at 8:42 AM Brian Gerst <brgerst@gmail.com> wrote:
>
> On Fri, Jun 26, 2020 at 1:30 PM Andy Lutomirski <luto@kernel.org> wrote:
> >
> > The SYSENTER frame setup was nonsense. It worked by accident
> > because the normal code into which the Xen asm jumped
> > (entry_SYSENTER_32/compat) threw away SP without touching the stack.
> > entry_SYSENTER_compat was recently modified such that it relied on
> > having a valid stack pointer, so now the Xen asm needs to invoke it
> > with a valid stack.
> >
> > Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
> > metal prologue.
> >
> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > Cc: Juergen Gross <jgross@suse.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: xen-devel@lists.xenproject.org
> > Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> > arch/x86/entry/entry_64_compat.S | 1 +
> > arch/x86/xen/xen-asm_64.S | 20 ++++++++++++++++----
> > 2 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
> > index 7b9d8150f652..381a6de7de9c 100644
> > --- a/arch/x86/entry/entry_64_compat.S
> > +++ b/arch/x86/entry/entry_64_compat.S
> > @@ -79,6 +79,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
> > pushfq /* pt_regs->flags (except IF = 0) */
> > pushq $__USER32_CS /* pt_regs->cs */
> > pushq $0 /* pt_regs->ip = 0 (placeholder) */
> > +SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
>
> This skips over the section that truncates the syscall number to
> 32-bits. The comments present some doubt that it is actually
> necessary, but the Xen path shouldn't differ from native. That code
> should be moved after this new label.
Whoops. I thought I caught that myself, but apparently not. I'll fix it.
>
> --
> Brian Gerst
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
2020-07-01 18:39 ` Andy Lutomirski
@ 2020-07-02 12:54 ` Thomas Gleixner
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Gleixner @ 2020-07-02 12:54 UTC (permalink / raw)
To: Andy Lutomirski, Brian Gerst
Cc: Juergen Gross, Stefano Stabellini, Andrew Cooper,
the arch/x86 maintainers, Linux Kernel Mailing List,
Andy Lutomirski, xen-devel, Boris Ostrovsky
Andy Lutomirski <luto@kernel.org> writes:
> On Wed, Jul 1, 2020 at 8:42 AM Brian Gerst <brgerst@gmail.com> wrote:
> > On Fri, Jun 26, 2020 at 1:30 PM Andy Lutomirski <luto@kernel.org> wrote:
>> >
>> > The SYSENTER frame setup was nonsense. It worked by accident
>> > because the normal code into which the Xen asm jumped
>> > (entry_SYSENTER_32/compat) threw away SP without touching the stack.
>> > entry_SYSENTER_compat was recently modified such that it relied on
>> > having a valid stack pointer, so now the Xen asm needs to invoke it
>> > with a valid stack.
>> >
>> > Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
>> > metal prologue.
>> >
>> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> > Cc: Juergen Gross <jgross@suse.com>
>> > Cc: Stefano Stabellini <sstabellini@kernel.org>
>> > Cc: xen-devel@lists.xenproject.org
>> > Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
>> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> > ---
>> > arch/x86/entry/entry_64_compat.S | 1 +
>> > arch/x86/xen/xen-asm_64.S | 20 ++++++++++++++++----
>> > 2 files changed, 17 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
>> > index 7b9d8150f652..381a6de7de9c 100644
>> > --- a/arch/x86/entry/entry_64_compat.S
>> > +++ b/arch/x86/entry/entry_64_compat.S
>> > @@ -79,6 +79,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
>> > pushfq /* pt_regs->flags (except IF = 0) */
>> > pushq $__USER32_CS /* pt_regs->cs */
>> > pushq $0 /* pt_regs->ip = 0 (placeholder) */
>> > +SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
>>
>> This skips over the section that truncates the syscall number to
>> 32-bits. The comments present some doubt that it is actually
>> necessary, but the Xen path shouldn't differ from native. That code
>> should be moved after this new label.
>
> Whoops. I thought I caught that myself, but apparently not. I'll fix it.
Darn. I already applied that lot. Can you please send a delta fix?
Thanks,
tglx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-07-02 12:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <cover.1593191971.git.luto@kernel.org>
2020-06-26 17:21 ` [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup Andy Lutomirski
2020-06-28 2:47 ` Boris Ostrovsky
2020-07-01 15:42 ` Brian Gerst
2020-07-01 18:39 ` Andy Lutomirski
2020-07-02 12:54 ` Thomas Gleixner
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).