Re: [PATCH for-next v2 2/2] xen/arm64: Place a speculation barrier following an ret instruction

[Julien Grall <julien@xxxxxxx>]

(+ Security)


On 17/03/2021 14:56, Bertrand Marquis wrote:
> Hi Julien,
Hi Bertrand,

> > On 13 Mar 2021, at 16:06, Julien Grall <julien@xxxxxxx> wrote:
> >
> > From: Julien Grall <jgrall@xxxxxxxxxx>
> >
> > Some CPUs can speculate past a RET instruction and potentially perform
> > speculative accesses to memory before processing the return.
> >
> > There is no known gadget available after the RET instruction today.
> > However some of the registers (such as in check_pending_guest_serror())
> > may contain a value provided by the guest.
> >
> > In order to harden the code, it would be better to add a speculation
> > barrier after each RET instruction. The performance impact is meant to
> > be negligeable as the speculation barrier is not meant to be
> > architecturally executed.
> >
> > Rather than manually inserting a speculation barrier, use a macro
> > which overrides the mnemonic RET and replace with RET + SB. We need to
> > use the opcode for RET to prevent any macro recursion.
> >
> > This patch is only covering the assembly code. C code would need to be
> > covered separately using the compiler support.
> >
> > This is part of the work to mitigate straight-line speculation.
> >
> > Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> The macro solution is definitely a great improvement compared to v1 and I could
> confirm the presence of the sb in the generated code.
Thanks for testing! It is indeed a lot nicer and less error-prone. We 
can thansk Jan for the idea as he originally introduced it on x86 :).
> I also think that the mitigation on arm32/v7 would be messy to do.
It is messy but not impossible :). Some of the assembly function could 
be rewritten in C to take advantage of the compiler mitigations.
I went through the paper again today. Straight-line mitigation only 
refers to unconditional control flow change (e.g. RET) on AArch64 Armv8.
A recent submission to LLVM seems to suggest that Armv7 and AArch32 
Armv8 is also affected [2].
So I think we only need to care of unconditional return instruction 
(e.g. mov pc, lr).
For conditional return instructions, they would be treated as spectre v2 
which we already mitigate.
> Shall we mark v7/aarch32 as not security supported ?
This would have consequence beyond just speculation. There might be 
processor out which are not affected by straight-line speculation and we 
would not issue any security update for them. So I am not in favor with 
this approach.
What we could consider is mentioning in SUPPORT.MD that speculation 
attack using Straight-Line speculation are not security support on Arm 
(the 64-bit is not fully mitigated).
Cheers,

[1] https://reviews.llvm.org/D92395

--
Julien Grall