lkml.org 
[lkml]   [2021]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 5/9] x86/alternative: Handle Jcc __x86_indirect_thunk_\reg
On Wed, Oct 13, 2021 at 5:41 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> Handle the rare cases where the compiler (clang) does an indirect
> conditional tail-call using:
>
> Jcc __x86_indirect_thunk_\reg

`Jcc.d32 __x86_indirect_thunk_\reg` might be clearer; otherwise
putting that in an assembler and assembling/disassembling produces the
2B instructions, which makes the below patch confusing. Ah, it is
stated in the comment added below.

>
> For the !RETPOLINE case this can be rewritten to fit the original (6
> byte) instruction like:
>
> Jncc.d8 1f
> JMP *%\reg
> NOP
> 1:
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> arch/x86/kernel/alternative.c | 38 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 34 insertions(+), 4 deletions(-)
>
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -378,7 +378,8 @@ static int emit_indirect(int op, int reg
> static int patch_retpoline(void *addr, struct insn *insn, u8 *bytes)
> {
> void (*target)(void);
> - int reg, i = 0;
> + int reg, ret, i = 0;
> + u8 op, cc;
>
> if (cpu_feature_enabled(X86_FEATURE_RETPOLINE))
> return -1;
> @@ -390,9 +391,34 @@ static int patch_retpoline(void *addr, s
> if (WARN_ON_ONCE(reg & ~0xf))
> return -1;
>
> - i = emit_indirect(insn->opcode.bytes[0], reg, bytes);
> - if (i < 0)
> - return i;
> + op = insn->opcode.bytes[0];
> +
> + /*
> + * Convert:
> + *
> + * Jcc.d32 __x86_indirect_thunk_\reg
> + *
> + * into:
> + *
> + * Jncc.d8 1f
> + * jmp *%\reg
> + * nop
> + * 1:
> + */
> + if (op == 0x0f && (insn->opcode.bytes[1] & 0xf0) == 0x80) {
> + cc = insn->opcode.bytes[1] & 0xf;
> + cc ^= 1; /* invert condition */
> +
> + bytes[i++] = 0x70 + cc; /* Jcc.d8 */
> + bytes[i++] = insn->length - 2;

Isn't `insn->length - 2` always 4 (in this case)? We could avoid
computing that at runtime I suspect if we just hardcoded it.

Either way, I've looked at the disassembly enough that this LGTM.
Thanks for the patch.

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> +
> + op = JMP32_INSN_OPCODE;
> + }
> +
> + ret = emit_indirect(op, reg, bytes + i);
> + if (ret < 0)
> + return ret;
> + i += ret;
>
> for (; i < insn->length;)
> bytes[i++] = BYTES_NOP1;
> @@ -423,6 +449,10 @@ void __init_or_module noinline apply_ret
> case JMP32_INSN_OPCODE:
> break;
>
> + case 0x0f: /* escape */
> + if (op2 >= 0x80 && op2 <= 0x8f)
> + break;
> + fallthrough;
> default:
> WARN_ON_ONCE(1);
> continue;
>
>


--
Thanks,
~Nick Desaulniers

\
 
 \ /
  Last update: 2021-10-13 22:13    [W:0.374 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site