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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210319032955.zdx6ihhprem5owbc@treble>
Date:   Thu, 18 Mar 2021 22:29:55 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     x86@...nel.org, jgross@...e.com, mbenes@...e.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 14/14] objtool,x86: Rewrite retpoline thunk calls

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@...radead.org>

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