[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 5/9] nospec: introduce evaluate_nospec
On 1/31/19 18:05, Jan Beulich wrote: >>>> On 29.01.19 at 15:43, <nmanthey@xxxxxxxxx> wrote: >> Since the L1TF vulnerability of Intel CPUs, loading hypervisor data into >> L1 cache is problemetic, because when hyperthreading is used as well, a >> guest running on the sibling core can leak this potentially secret data. >> >> To prevent these speculative accesses, we block speculation after >> accessing the domain property field by adding lfence instructions. This >> way, the CPU continues executing and loading data only once the condition >> is actually evaluated. >> >> As the macros are typically used in if statements, the lfence has to come >> in a compatible way. Therefore, a function that returns true after an >> lfence instruction is introduced. To protect both branches after a >> conditional, an lfence instruction has to be added for the two branches. >> To be able to block speculation after several evalauations, the generic >> barrier macro block_speculation is also introduced. >> >> As the L1TF vulnerability is only present on the x86 architecture, the >> macros will not use the lfence instruction on other architectures and the >> protection is disabled during compilation. By default, the lfence >> instruction is not present either. Only when a L1TF vulnerable platform >> is detected, the lfence instruction is patched in via alterantive patching. >> >> Introducing the lfence instructions catches a lot of potential leaks with >> a simple unintrusive code change. During performance testing, we did not >> notice performance effects. >> >> Signed-off-by: Norbert Manthey <nmanthey@xxxxxxxxx> > Looks okay to me now, but I'm going to wait with giving an ack > until perhaps others have given comments, as some of this > was not entirely uncontroversial. There are a few cosmetic > issues left though: > >> @@ -64,6 +65,33 @@ static inline unsigned long >> array_index_mask_nospec(unsigned long index, >> #define array_access_nospec(array, index) \ >> (array)[array_index_nospec(index, ARRAY_SIZE(array))] >> >> +/* >> + * Allow to insert a read memory barrier into conditionals >> + */ > Here and below, please make single line comments really be > single lines. Will fix. > >> +#if defined(CONFIG_X86) && defined(CONFIG_HVM) >> +static inline bool arch_barrier_nospec_true(void) { > The brace belongs on its own line. Will fix. > >> + alternative("", "lfence", X86_FEATURE_SC_L1TF_VULN); >> + return true; >> +} >> +#else >> +static inline bool arch_barrier_nospec_true(void) { return true; } > This could be avoided if you placed the #if inside the > function body. I will move the #if inside. > >> +#endif >> + >> +/* >> + * Allow to protect evaluation of conditional with respect to speculation >> on x86 >> + */ >> +#ifndef CONFIG_X86 > Why is this conditional different from the one above? You are right, the two defines should be equal. > >> +#define evaluate_nospec(condition) (condition) >> +#else >> +#define evaluate_nospec(condition) \ >> + ((condition) ? arch_barrier_nospec_true() : !arch_barrier_nospec_true()) >> +#endif >> + >> +/* >> + * Allow to block speculative execution in generic code >> + */ >> +#define block_speculation() (void)arch_barrier_nospec_true() > Missing an outer pair of parentheses. Will add them. Best, Norbert > > Jan > > Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich Ust-ID: DE 289 237 879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |