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@vger.kernel.org Fixes: babdde2698d4 ("x86: Implement array_index_mask_nospec") Cc: Thomas Gleixner tglx@linutronix.de Cc: Peter Zijlstra peterz@infradead.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Ingo Molnar mingo@kernel.org Reported-by: Mark Rutland mark.rutland@arm.com Signed-off-by: Dan Williams dan.j.williams@intel.com --- 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; }