Re: [PATCH v2 14/14] objtool,x86: Rewrite retpoline thunk calls

[Josh Poimboeuf]

On Thu, Mar 18, 2021 at 06:11:17PM +0100, Peter Zijlstra wrote:
> When the compiler emits: "CALL __x86_indirect_thunk_\reg" for an
> indirect call, have objtool rewrite it to:
> 
> 	ALTERNATIVE "call __x86_indirect_thunk_\reg",
> 		    "call *%reg", ALT_NOT(X86_FEATURE_RETPOLINE)
> 
> Additionally, in order to not emit endless identical
> .altinst_replacement chunks, use a global symbol for them, see
> __x86_indirect_alt_*.
> 
> This also avoids objtool from having to do code generation.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>

This is better than I expected.  Nice workaround for not generating
code.

> +.macro ALT_THUNK reg
> +
> +	.align 1
> +
> +SYM_FUNC_START_NOALIGN(__x86_indirect_alt_call_\reg)
> +	ANNOTATE_RETPOLINE_SAFE
> +1:	call	*%\reg
> +2:	.skip	5-(2b-1b), 0x90
> +SYM_FUNC_END(__x86_indirect_alt_call_\reg)
> +
> +SYM_FUNC_START_NOALIGN(__x86_indirect_alt_jmp_\reg)
> +	ANNOTATE_RETPOLINE_SAFE
> +1:	jmp	*%\reg
> +2:	.skip	5-(2b-1b), 0x90
> +SYM_FUNC_END(__x86_indirect_alt_jmp_\reg)

This mysterious code needs a comment.  Shouldn't it be in
.altinstr_replacement or something?

Also doesn't the alternative code already insert nops?

> +int arch_rewrite_retpoline(struct objtool_file *file,
> +			   struct instruction *insn,
> +			   struct reloc *reloc)
> +{
> +	struct symbol *sym;
> +	char name[32] = "";
> +
> +	if (!strcmp(insn->sec->name, ".text.__x86.indirect_thunk"))
> +		return 0;
> +
> +	sprintf(name, "__x86_indirect_alt_%s_%s",
> +		insn->type == INSN_JUMP_DYNAMIC ? "jmp" : "call",
> +		reloc->sym->name + 21);
> +
> +	sym = find_symbol_by_name(file->elf, name);
> +	if (!sym) {
> +		sym = elf_create_undef_symbol(file->elf, name);
> +		if (!sym) {
> +			WARN("elf_create_undef_symbol");
> +			return -1;
> +		}
> +	}
> +
> +	elf_add_alternative(file->elf, insn, sym,
> +			    ALT_NOT(X86_FEATURE_RETPOLINE), 5, 5);
> +
> +	return 0;
> +}

Need to propagate the error.

-- 
Josh