Mark notes that gcc optimization passes have the potential to elide necessary invocations of this instruction sequence, so include an optimization barrier. > I think that either way, we have a potential problem if the compiler > generates a branch dependent on the result of validate_index_nospec(). > > In that case, we could end up with codegen approximating: > > bool safe = false; > > if (idx < bound) { > idx = array_index_nospec(idx, bound); > safe = true; > } > > // this branch can be mispredicted > if (safe) { > foo = array[idx]; > } > > ... and thus we lose the nospec protection. I see GCC do this at -O0, but so far I haven't tricked it into doing this at -O1 or above. Regardless, I worry this is fragile -- GCC *can* generate code as per the above, even if it's unlikely to. Cc: <stable@xxxxxxxxxxxxxxx> Fixes: babdde2698d4 ("x86: Implement array_index_mask_nospec") Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> Cc: Ingo Molnar <mingo@xxxxxxxxxx> Reported-by: Mark Rutland <mark.rutland@xxxxxxx> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> --- arch/x86/include/asm/barrier.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h index 042b5e892ed1..41f7435c84a7 100644 --- a/arch/x86/include/asm/barrier.h +++ b/arch/x86/include/asm/barrier.h @@ -38,10 +38,11 @@ static inline unsigned long array_index_mask_nospec(unsigned long index, { unsigned long mask; - asm ("cmp %1,%2; sbb %0,%0;" + asm volatile ("cmp %1,%2; sbb %0,%0;" :"=r" (mask) :"g"(size),"r" (index) :"cc"); + barrier(); return mask; }