[PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

[PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

[Andy Lutomirski]

Xen doesn't call start_secondary.

Cc: stable@vger.kernel.org
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---

espfix still doesn't seem to work on Xen (it goes boom in some way that
I don't understand right now), but initializing all CPUs instead of just
one of them seems like a good start.

ISTM the right fix is probably to shove the espfix logic into
native_iret and to tweak the paravirt logic so that native_iret always
gets invoked.  I suspect that Xen will need its own implementation of
espfix64 in the hypervisor and that, ultimately, someone may want to
stop initializing espfix64 at all on Xen guests.

 arch/x86/xen/smp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 7005974..eea9bcc 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -23,6 +23,7 @@
 #include <asm/desc.h>
 #include <asm/pgtable.h>
 #include <asm/cpu.h>
+#include <asm/espfix.h>
 
 #include <xen/interface/xen.h>
 #include <xen/interface/vcpu.h>
@@ -85,6 +86,13 @@ static void cpu_bringup(void)
 
 	xen_setup_cpu_clockevents();
 
+	/*
+	 * Enable the espfix hack for this CPU
+	 */
+#ifdef CONFIG_X86_ESPFIX64
+	init_espfix_ap();
+#endif
+
 	notify_cpu_starting(cpu);
 
 	set_cpu_online(cpu, true);
-- 
1.9.3

Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

[Konrad Rzeszutek Wilk]

On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
> Xen doesn't call start_secondary.

Duh!
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> ---
> 
> espfix still doesn't seem to work on Xen (it goes boom in some way that
> I don't understand right now), but initializing all CPUs instead of just
> one of them seems like a good start.
> 
> ISTM the right fix is probably to shove the espfix logic into
> native_iret and to tweak the paravirt logic so that native_iret always
> gets invoked.  I suspect that Xen will need its own implementation of
> espfix64 in the hypervisor and that, ultimately, someone may want to
> stop initializing espfix64 at all on Xen guests.

I think just disallowing would be preferrable.

Thanks for posting this patch!
> 
>  arch/x86/xen/smp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index 7005974..eea9bcc 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -23,6 +23,7 @@
>  #include <asm/desc.h>
>  #include <asm/pgtable.h>
>  #include <asm/cpu.h>
> +#include <asm/espfix.h>
>  
>  #include <xen/interface/xen.h>
>  #include <xen/interface/vcpu.h>
> @@ -85,6 +86,13 @@ static void cpu_bringup(void)
>  
>  	xen_setup_cpu_clockevents();
>  
> +	/*
> +	 * Enable the espfix hack for this CPU
> +	 */
> +#ifdef CONFIG_X86_ESPFIX64
> +	init_espfix_ap();
> +#endif
> +
>  	notify_cpu_starting(cpu);
>  
>  	set_cpu_online(cpu, true);
> -- 
> 1.9.3
> 

Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

[Andy Lutomirski]

On Tue, Jul 15, 2014 at 8:38 AM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
>> Xen doesn't call start_secondary.
>
> Duh!
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>> ---
>>
>> espfix still doesn't seem to work on Xen (it goes boom in some way that
>> I don't understand right now), but initializing all CPUs instead of just
>> one of them seems like a good start.
>>
>> ISTM the right fix is probably to shove the espfix logic into
>> native_iret and to tweak the paravirt logic so that native_iret always
>> gets invoked.  I suspect that Xen will need its own implementation of
>> espfix64 in the hypervisor and that, ultimately, someone may want to
>> stop initializing espfix64 at all on Xen guests.
>
> I think just disallowing would be preferrable.

Disabling what?

Sorry, my flu-addled brain needs more clarity.  I'm currently working
on a patch on top of this one to move all of the espfix64 invocation
logic into native_iret, which will have the effect of preventing it
from being used on Xen.

Is that what you mean?

--Andy

>
> Thanks for posting this patch!
>>
>>  arch/x86/xen/smp.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
>> index 7005974..eea9bcc 100644
>> --- a/arch/x86/xen/smp.c
>> +++ b/arch/x86/xen/smp.c
>> @@ -23,6 +23,7 @@
>>  #include <asm/desc.h>
>>  #include <asm/pgtable.h>
>>  #include <asm/cpu.h>
>> +#include <asm/espfix.h>
>>
>>  #include <xen/interface/xen.h>
>>  #include <xen/interface/vcpu.h>
>> @@ -85,6 +86,13 @@ static void cpu_bringup(void)
>>
>>       xen_setup_cpu_clockevents();
>>
>> +     /*
>> +      * Enable the espfix hack for this CPU
>> +      */
>> +#ifdef CONFIG_X86_ESPFIX64
>> +     init_espfix_ap();
>> +#endif
>> +
>>       notify_cpu_starting(cpu);
>>
>>       set_cpu_online(cpu, true);
>> --
>> 1.9.3
>>



-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

[Konrad Rzeszutek Wilk]

On Tue, Jul 15, 2014 at 08:44:39AM -0700, Andy Lutomirski wrote:
> On Tue, Jul 15, 2014 at 8:38 AM, Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
> > On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
> >> Xen doesn't call start_secondary.
> >
> > Duh!
> >>
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> >> ---
> >>
> >> espfix still doesn't seem to work on Xen (it goes boom in some way that
> >> I don't understand right now), but initializing all CPUs instead of just
> >> one of them seems like a good start.
> >>
> >> ISTM the right fix is probably to shove the espfix logic into
> >> native_iret and to tweak the paravirt logic so that native_iret always
> >> gets invoked.  I suspect that Xen will need its own implementation of
> >> espfix64 in the hypervisor and that, ultimately, someone may want to
> >> stop initializing espfix64 at all on Xen guests.
> >
> > I think just disallowing would be preferrable.
> 
> Disabling what?
> 
> Sorry, my flu-addled brain needs more clarity.  I'm currently working
> on a patch on top of this one to move all of the espfix64 invocation
> logic into native_iret, which will have the effect of preventing it
> from being used on Xen.
> 
> Is that what you mean?

Yes. I presume the logic to deal with the bits losing information
has to be dealt in the Xen case somehow. Peter asked whether the 
Xen IRET handles a 16-bit stack segment - and if it restores all of the
RSP then we are OK.

I don't have yet that information and my brain is a in low-power right
now (-ENOSLEEP).

Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

[Boris Ostrovsky]

On 07/15/2014 11:38 AM, Konrad Rzeszutek Wilk wrote:
> On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
>> Xen doesn't call start_secondary.
> Duh!
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>> ---
>>
>> espfix still doesn't seem to work on Xen (it goes boom in some way that
>> I don't understand right now), but initializing all CPUs instead of just
>> one of them seems like a good start.
>>
>> ISTM the right fix is probably to shove the espfix logic into
>> native_iret and to tweak the paravirt logic so that native_iret always
>> gets invoked.  I suspect that Xen will need its own implementation of
>> espfix64 in the hypervisor and that, ultimately, someone may want to
>> stop initializing espfix64 at all on Xen guests.
> I think just disallowing would be preferrable.

I've been looking at sigreturn_64 and it seems to be crashing dom0 (with 
both mine and your patches). In kprobe_int3_handler().


-boris

>
> Thanks for posting this patch!
>>   arch/x86/xen/smp.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
>> index 7005974..eea9bcc 100644
>> --- a/arch/x86/xen/smp.c
>> +++ b/arch/x86/xen/smp.c
>> @@ -23,6 +23,7 @@
>>   #include <asm/desc.h>
>>   #include <asm/pgtable.h>
>>   #include <asm/cpu.h>
>> +#include <asm/espfix.h>
>>   
>>   #include <xen/interface/xen.h>
>>   #include <xen/interface/vcpu.h>
>> @@ -85,6 +86,13 @@ static void cpu_bringup(void)
>>   
>>   	xen_setup_cpu_clockevents();
>>   
>> +	/*
>> +	 * Enable the espfix hack for this CPU
>> +	 */
>> +#ifdef CONFIG_X86_ESPFIX64
>> +	init_espfix_ap();
>> +#endif
>> +
>>   	notify_cpu_starting(cpu);
>>   
>>   	set_cpu_online(cpu, true);
>> -- 
>> 1.9.3
>>

Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

[Andy Lutomirski]

On Tue, Jul 15, 2014 at 8:45 AM, Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
> On 07/15/2014 11:38 AM, Konrad Rzeszutek Wilk wrote:
>>
>> On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
>>>
>>> Xen doesn't call start_secondary.
>>
>> Duh!
>>>
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>>> ---
>>>
>>> espfix still doesn't seem to work on Xen (it goes boom in some way that
>>> I don't understand right now), but initializing all CPUs instead of just
>>> one of them seems like a good start.
>>>
>>> ISTM the right fix is probably to shove the espfix logic into
>>> native_iret and to tweak the paravirt logic so that native_iret always
>>> gets invoked.  I suspect that Xen will need its own implementation of
>>> espfix64 in the hypervisor and that, ultimately, someone may want to
>>> stop initializing espfix64 at all on Xen guests.
>>
>> I think just disallowing would be preferrable.
>
>
> I've been looking at sigreturn_64 and it seems to be crashing dom0 (with
> both mine and your patches). In kprobe_int3_handler().

You need:

http://lkml.kernel.org/g/c4e339882c121aa76254f2adde3fcbdf502faec2.1405099506.git.luto@amacapital.net

The newer version of sigreturn_32 that I pushed is a much better test
-- it tests the 64-bit cases (yay thunks!) and works on kernels
without my SS sigcontext fix.

--Andy

Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

[Boris Ostrovsky]

On 07/15/2014 11:54 AM, Andy Lutomirski wrote:
> On Tue, Jul 15, 2014 at 8:45 AM, Boris Ostrovsky
> <boris.ostrovsky@oracle.com> wrote:
>> On 07/15/2014 11:38 AM, Konrad Rzeszutek Wilk wrote:
>>> On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
>>>> Xen doesn't call start_secondary.
>>> Duh!
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>>>> ---
>>>>
>>>> espfix still doesn't seem to work on Xen (it goes boom in some way that
>>>> I don't understand right now), but initializing all CPUs instead of just
>>>> one of them seems like a good start.
>>>>
>>>> ISTM the right fix is probably to shove the espfix logic into
>>>> native_iret and to tweak the paravirt logic so that native_iret always
>>>> gets invoked.  I suspect that Xen will need its own implementation of
>>>> espfix64 in the hypervisor and that, ultimately, someone may want to
>>>> stop initializing espfix64 at all on Xen guests.
>>> I think just disallowing would be preferrable.
>>
>> I've been looking at sigreturn_64 and it seems to be crashing dom0 (with
>> both mine and your patches). In kprobe_int3_handler().
> You need:
>
> http://lkml.kernel.org/g/c4e339882c121aa76254f2adde3fcbdf502faec2.1405099506.git.luto@amacapital.net
>
> The newer version of sigreturn_32 that I pushed is a much better test
> -- it tests the 64-bit cases (yay thunks!) and works on kernels
> without my SS sigcontext fix.

Yes, that does it. At least we don't have yet another failure mode with 
this, which was the biggest concern.

Thanks.

-boris

Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

[Andy Lutomirski]

On Tue, Jul 15, 2014 at 9:05 AM, Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
> On 07/15/2014 11:54 AM, Andy Lutomirski wrote:
>>
>> On Tue, Jul 15, 2014 at 8:45 AM, Boris Ostrovsky
>> <boris.ostrovsky@oracle.com> wrote:
>>>
>>> On 07/15/2014 11:38 AM, Konrad Rzeszutek Wilk wrote:
>>>>
>>>> On Tue, Jul 15, 2014 at 08:26:41AM -0700, Andy Lutomirski wrote:
>>>>>
>>>>> Xen doesn't call start_secondary.
>>>>
>>>> Duh!
>>>>>
>>>>> Cc: stable@vger.kernel.org
>>>>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>>>>> ---
>>>>>
>>>>> espfix still doesn't seem to work on Xen (it goes boom in some way that
>>>>> I don't understand right now), but initializing all CPUs instead of
>>>>> just
>>>>> one of them seems like a good start.
>>>>>
>>>>> ISTM the right fix is probably to shove the espfix logic into
>>>>> native_iret and to tweak the paravirt logic so that native_iret always
>>>>> gets invoked.  I suspect that Xen will need its own implementation of
>>>>> espfix64 in the hypervisor and that, ultimately, someone may want to
>>>>> stop initializing espfix64 at all on Xen guests.
>>>>
>>>> I think just disallowing would be preferrable.
>>>
>>>
>>> I've been looking at sigreturn_64 and it seems to be crashing dom0 (with
>>> both mine and your patches). In kprobe_int3_handler().
>>
>> You need:
>>
>>
>> http://lkml.kernel.org/g/c4e339882c121aa76254f2adde3fcbdf502faec2.1405099506.git.luto@amacapital.net
>>
>> The newer version of sigreturn_32 that I pushed is a much better test
>> -- it tests the 64-bit cases (yay thunks!) and works on kernels
>> without my SS sigcontext fix.
>
>
> Yes, that does it. At least we don't have yet another failure mode with
> this, which was the biggest concern.

I updated the sigreturn test again -- the new version exercises the
double-fault case, too.  It passes on a non-Xen kernel with the
krpobes fix.

--Andy

>
> Thanks.
>
> -boris
>



-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

[H. Peter Anvin]

On 07/15/2014 08:38 AM, Konrad Rzeszutek Wilk wrote:
> 
> I think just disallowing would be preferrable.
> 

So I have yet to hear anyone talk about what the Xen PV IRET does for
16-bit stack segments.  Can we get the answer for that, please?

	-hpa

Re: [PATCH] x86_64,xen,espfix: Initialize espfix on secondary CPUs

[Andy Lutomirski]

On Tue, Jul 15, 2014 at 8:50 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 07/15/2014 08:38 AM, Konrad Rzeszutek Wilk wrote:
>>
>> I think just disallowing would be preferrable.
>>
>
> So I have yet to hear anyone talk about what the Xen PV IRET does for
> 16-bit stack segments.  Can we get the answer for that, please?
>

If I'm reading the right thing, it does:

 59 .Lforce_iret:
60         /* Mimic SYSRET behavior. */
61         movq  8(%rsp),%rcx            # RIP
62         movq  24(%rsp),%r11           # RFLAGS
63         ALIGN
64 /* No special register assumptions. */
65 iret_exit_to_guest:
66         addq  $8,%rsp
67 .Lft0:  iretq

while running on Xen's stack.  IOW, it leaks bits 31:16 of the Xen
stack, and there's nothing the guest can do about it short of using a
user-mode trampoline or disabling 16-bit stack segments entirely.

See:

http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/x86/x86_64/entry.S;h=a3ed216b390c2e87a21ff377850ee34ee7f2bc74;hb=HEAD

and (search for do_iret):

http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/x86/traps.c;h=677074b4e628ed99d407b1045d859355e590d604;hb=HEAD

>         -hpa
>
>



-- 
Andy Lutomirski
AMA Capital Management, LLC