Messages in this thread Patch in this message | | | Date | Tue, 19 Oct 2021 13:37:09 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 4/9] x86/alternative: Implement .retpoline_sites support |
| |
On Wed, Oct 13, 2021 at 01:52:59PM -0700, Josh Poimboeuf wrote:
> BTW, CALL_NOSPEC results in a retpoline site in .altinstr_replacement: > > Relocation section [40] '.rela.retpoline_sites' for section [39] '.retpoline_sites' at offset 0x8d28 contains 1 entry: > Offset Type Value Addend Name > 000000000000000000 X86_64_PC32 000000000000000000 +10 .altinstr_replacement > > Which I assume we don't want.
(I missed this initially, and just independently rediscovered it)
In principle this problem affects static_call_list, the __sanitizer_cov_ and __fentry__ and now retpoline_sites.
Granted, it seems really unlikely to find __fentry__ or __sanitizer_cov_ references in alternatives, but it should be trivial to manually create one.
I'm thinking we want to exclude all those when found in .altinstr_replacement, right? It just doesn't make sense to rewrite replacement text.
How is something like the below? (I'm not completely happy with it, but I couldn't think of something better either).
--- --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1039,14 +1039,39 @@ static void remove_insn_ops(struct instr } } +#define DEST_RETPOLINE ((void *)-1L) + static void add_call_dest(struct objtool_file *file, struct instruction *insn, struct symbol *dest, bool sibling) { struct reloc *reloc = insn_reloc(file, insn); - insn->call_dest = dest; - if (!dest) + if (dest != DEST_RETPOLINE) { + insn->call_dest = dest; + if (!dest ) + return; + } + + /* + * Whatever stack impact regular CALLs have, should be undone + * by the RETURN of the called function. + * + * Annotated intra-function calls retain the stack_ops but + * are converted to JUMP, see read_intra_function_calls(). + */ + remove_insn_ops(insn); + + /* + * Whatever we do, do not rewrite replacement text. + */ + if (!strcmp(insn->sec->name, ".altinstr_replacement")) + return; + + if (dest == DEST_RETPOLINE) { + list_add_tail(&insn->call_node, + &file->retpoline_call_list); return; + } if (insn->call_dest->static_call_tramp) { list_add_tail(&insn->call_node, @@ -1091,15 +1116,6 @@ static void add_call_dest(struct objtool list_add_tail(&insn->mcount_loc_node, &file->mcount_loc_list); } - - /* - * Whatever stack impact regular CALLs have, should be undone - * by the RETURN of the called function. - * - * Annotated intra-function calls retain the stack_ops but - * are converted to JUMP, see read_intra_function_calls(). - */ - remove_insn_ops(insn); } /* @@ -1133,10 +1149,9 @@ static int add_jump_destinations(struct else insn->type = INSN_JUMP_DYNAMIC_CONDITIONAL; - list_add_tail(&insn->call_node, - &file->retpoline_call_list); - insn->retpoline_safe = true; + + add_call_dest(file, insn, DEST_RETPOLINE, true); continue; } else if (insn->func) { /* internal or external sibling call (with reloc) */ @@ -1272,12 +1287,7 @@ static int add_call_destinations(struct insn->type = INSN_CALL_DYNAMIC; insn->retpoline_safe = true; - list_add_tail(&insn->call_node, - &file->retpoline_call_list); - - remove_insn_ops(insn); - continue; - + add_call_dest(file, insn, DEST_RETPOLINE, false); } else add_call_dest(file, insn, reloc->sym, false); }
| |