lkml.org 
[lkml]   [2017]   [Apr]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] x86/amd: don't set X86_BUG_SYSRET_SS_ATTRS if forced to zero
From
Date
On 25/04/17 20:24, Borislav Petkov wrote:
> On Tue, Apr 25, 2017 at 08:00:14PM +0200, Juergen Gross wrote:
>> When running as Xen pv guest X86_BUG_SYSRET_SS_ATTRS must not be set
>> on AMD cpus. Xen will disable this via setup_clear_cpu_cap(), so test
>> cpu_caps_cleared to not have disabled this bit.
>>
>> This bug/feature bit is kind of special as it will be used very early
>> when switching threads. Setting the bit and clearing it a little bit
>> later leaves a critical window where things can go wrong. This time
>> window has enlarged a little bit by using setup_clear_cpu_cap() instead
>> of the hypervisor's set_cpu_features callback. It seems this larger
>> window now makes it rather easy to hit the problem.
>>
>> The proper solution is to never set the bit in case of Xen.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> arch/x86/kernel/cpu/amd.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>> index c36140d788fe..f659b6f534b7 100644
>> --- a/arch/x86/kernel/cpu/amd.c
>> +++ b/arch/x86/kernel/cpu/amd.c
>> @@ -800,7 +800,9 @@ static void init_amd(struct cpuinfo_x86 *c)
>> set_cpu_cap(c, X86_FEATURE_3DNOWPREFETCH);
>>
>> /* AMD CPUs don't reset SS attributes on SYSRET */
>> - set_cpu_bug(c, X86_BUG_SYSRET_SS_ATTRS);
>> + if (!test_bit(X86_BUG_SYSRET_SS_ATTRS,
>> + (unsigned long *)cpu_caps_cleared))
>> + set_cpu_bug(c, X86_BUG_SYSRET_SS_ATTRS);
>> }
>
> Hold on, AFAICT we have this order:
>
> c_init() -> init_amd() sets X86_BUG_SYSRET_SS_ATTRS

And what happens when there is a scheduling event right here?
__switch_to() will see X86_BUG_SYSRET_SS_ATTRS set and take a wrong
path.

> init_hypervisor->x86_hyper->set_cpu_features-> clear_cpu_bug(c, X86_BUG_SYSRET_SS_ATTRS);

And now (with setup_clear_cpu_cap()) the bit is cleared a little bit
later. And all should be good. But it isn't.

>
> and all is good.
>
> And I remember seeing a patchset doing some xen cpuid cleanup so I'm
> assuming you're doing setup_clear_cpu_cap() now? And now we have to wag
> the dog?

No, now the time window with X86_BUG_SYSRET_SS_ATTRS set is so long we
actually see the problem happening.


Juergen

\
 
 \ /
  Last update: 2017-04-25 20:36    [W:0.084 / U:0.372 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site