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

2018-02-06 Thread Andrew Cooper
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 
>> 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.

~Andrew

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

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

2018-02-06 Thread Zhenzhong Duan

在 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 

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.


(IOW, the logic here is deliberate, and has a specific purpose.)

Ok, Just ignore the noise if it's deliberate.

--
thanks
zduan


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

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

2018-02-06 Thread 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 

Why?  What improvement is this intended to give?

(IOW, the logic here is deliberate, and has a specific purpose.)

~Andrew

> ---
>  xen/arch/x86/spec_ctrl.c |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
> index f10ffbf..ab4b244 100644
> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -247,10 +247,10 @@ void __init init_speculation_mitigations(void)
>  
>  /*
>   * If there are still no thunk preferences, the compiled default is
> - * actually retpoline, and it is better than nothing.
> + * actually retpoline, and it is better than nothing if it's retpoline 
> safe.
>   */
>  if ( thunk == THUNK_DEFAULT )
> -thunk = THUNK_RETPOLINE;
> +thunk = retpoline_safe() ? THUNK_RETPOLINE : THUNK_JMP;
>  
>  /* Apply the chosen settings. */
>  if ( thunk == THUNK_LFENCE )


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

[Xen-devel] [PATCH] Choose retpoline only when it is safe to use

2018-02-06 Thread Zhenzhong Duan
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 
---
 xen/arch/x86/spec_ctrl.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index f10ffbf..ab4b244 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -247,10 +247,10 @@ void __init init_speculation_mitigations(void)
 
 /*
  * If there are still no thunk preferences, the compiled default is
- * actually retpoline, and it is better than nothing.
+ * actually retpoline, and it is better than nothing if it's retpoline 
safe.
  */
 if ( thunk == THUNK_DEFAULT )
-thunk = THUNK_RETPOLINE;
+thunk = retpoline_safe() ? THUNK_RETPOLINE : THUNK_JMP;
 
 /* Apply the chosen settings. */
 if ( thunk == THUNK_LFENCE )
-- 
1.7.3

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