Skip to content

Commit

Permalink
x86/nospec: Fix evaluate_nospec() code generation under Clang
Browse files Browse the repository at this point in the history
It turns out that evaluate_nospec() code generation is not safe under Clang.
Given:

  void eval_nospec_test(int x)
  {
      if ( evaluate_nospec(x) )
          asm volatile ("nop #true" ::: "memory");
      else
          asm volatile ("nop #false" ::: "memory");
  }

Clang emits:

  <eval_nospec_test>:
         0f ae e8                lfence
         85 ff                   test   %edi,%edi
         74 02                   je     <eval_nospec_test+0x9>
         90                      nop
         c3                      ret
         90                      nop
         c3                      ret

which is not safe because the lfence has been hoisted above the conditional
jump.  Clang concludes that both barrier_nospec_true()'s have identical side
effects and can safely be merged.

Clang can be persuaded that the side effects are different if there are
different comments in the asm blocks.  This is fragile, but no more fragile
that other aspects of this construct.

Introduce barrier_nospec_false() with a separate internal comment to prevent
Clang merging it with barrier_nospec_true() despite the otherwise-identical
content.  The generated code now becomes:

  <eval_nospec_test>:
         85 ff                   test   %edi,%edi
         74 05                   je     <eval_nospec_test+0x9>
         0f ae e8                lfence
         90                      nop
         c3                      ret
         0f ae e8                lfence
         90                      nop
         c3                      ret

which has the correct number of lfence's, and in the correct place.

Link: llvm/llvm-project#55084
Signed-off-by: Andrew Cooper <[email protected]>
Reviewed-by: Roger Pau Monné <[email protected]>
Reviewed-by: Jan Beulich <[email protected]>
master commit: bc3c133
master date: 2023-03-24 12:16:31 +0000
  • Loading branch information
andyhhp authored and jbeulich committed Mar 31, 2023
1 parent 6f2d89d commit 00aa5c9
Showing 1 changed file with 13 additions and 2 deletions.
15 changes: 13 additions & 2 deletions xen/arch/x86/include/asm/nospec.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,26 @@
static always_inline bool barrier_nospec_true(void)
{
#ifdef CONFIG_SPECULATIVE_HARDEN_BRANCH
alternative("lfence", "", X86_FEATURE_SC_NO_BRANCH_HARDEN);
alternative("lfence #nospec-true", "", X86_FEATURE_SC_NO_BRANCH_HARDEN);
#endif
return true;
}

static always_inline bool barrier_nospec_false(void)
{
#ifdef CONFIG_SPECULATIVE_HARDEN_BRANCH
alternative("lfence #nospec-false", "", X86_FEATURE_SC_NO_BRANCH_HARDEN);
#endif
return false;
}

/* Allow to protect evaluation of conditionals with respect to speculation */
static always_inline bool evaluate_nospec(bool condition)
{
return condition ? barrier_nospec_true() : !barrier_nospec_true();
if ( condition )
return barrier_nospec_true();
else
return barrier_nospec_false();
}

/* Allow to block speculative execution in generic code */
Expand Down

0 comments on commit 00aa5c9

Please sign in to comment.