|
On 06/02/18 12:30, Zhenzhong Duan
wrote:
在 2018/2/6 19:56, Andrew Cooper 写道:
On 06/02/18 11:41, Zhenzhong Duan
wrote:
On 2018/2/6 18:50, Andrew Cooper
wrote:
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.
Understood.
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,
Sorry, sent from mobile.
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.
That does make sense for AMD. But what about Intel hardware
which has (!cpu_has_lfence_dispatch &&
!retpoline_safe() &&
!boot_cpu_has(X86_FEATURE_IBRSB)), should we prefer
THUNK_RETPOLINE?
Yes, and we do end up choosing RETPOLINE in those circumstances,
as we hit the default case you originally tried to patch.
If that's the case, I think we need another patch like below.
@@ -223,7 +223,7 @@ void __init init_speculation_mitigations(void)
*/
else if ( retpoline_safe() )
thunk = THUNK_RETPOLINE;
- else
+ else if ( boot_cpu_has(X86_FEATURE_IBRSB) )
ibrs = true;
}
/* Without compiler thunk support, use IBRS if available.
*/
As ibrs was set to true when !boot_cpu_has(X86_FEATURE_IBRSB) with
current code, then thunk was set to THUNK_JMP rather than
THUNK_RETPOLINE.
You are correct. This is a side effect of an optimisation which was
requested during review. I'll draft a patch.
~Andrew
|
