Re: [Xen-devel] 回复: Re: [PATCH] Choose retpoline only when it is safe to use

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/02/18 10:29, zhenzhong.duan wrote:

2018年2月6日 17:20于 Andrew Cooper <andrew.cooper3@xxxxxxxxxx>写道：
>
> On 06/02/2018 09:13, Zhenzhong Duan wrote:
> > 在 2018/2/6 16:59, Andrew Cooper 写道:
> >> On 06/02/2018 08:43, Zhenzhong Duan wrote:
> >>> When ( ibrs && thunk == THUNK_DEFAULT && !retpoline_safe() ) is true,
> >>> thunk is set to THUNK_JMP rather than THUNK_RETPOLINE.
> >>>
> >>> When (!ibrs && thunk == THUNK_DEFAULT && !retpoline_safe() ) is true,
> >>> we should do the same.
> >>>
> >>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@xxxxxxxxxx>
> >> Why?  What improvement is this intended to give?
> > No improvement, I just feel if retpoline isn't safe, THUNK_JMP is
> > better and safer.
> > Above first check is working that way.
>
> If your only two choices are unsafe repoline or plain jumps, then unsafe
> repoline is far far far safer.
>
> Its unsafe properties only kick in on an RSB underflow, and an attacker
> would have to do call-depths analysis of the running binary to identify
> which rets to attempt to poison.
>
Thanks for explaining.
So, for a retpoline safe processor, it just stop using RSB when it's empty to avoid underflow?

The qualification of whether a processor is retpoline-safe or not is whether an RSB underflow causes a BTB lookup (unsafe) or not (safe).

RSB underflows will always occur; you cannot get rid of them, but in most cases (i.e. pre-Skylake) they don't fall back to using a prediction source which can be poisoned by an attacker.

Another question:

if (opt_thunk == THUNK_DEFAULT && opt_ibrs == -1 &&
CONFIG_INDIRECT_THUNK && !cpu_has_lfence_dispatch && !retpoline_safe())
results in "thunk = THUNK_JMP" regardless of the value of "boot_cpu_has(X86_FEATURE_IBRSB)"

Your formatting is hard to follow, but cpu_has_lfence_dispatch can only be false when virtualised under an SP2-unaware hypervisor on AMD hardware, at which point retpoline_safe() will return true.  Also, AMD don't have IBRS in microcode and their future plans don't appear to involve using that particular CPUID bit.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel