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.coop...@citrix.com >>> <mailto:andrew.coop...@citrix.com>>写道: >>> > >>> > 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.d...@oracle.com >>> <mailto:zhenzhong.d...@oracle.com>> >>> > >> 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. ~Andrew
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel