From: Marco Elver <elver@google.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: jpoimboe@redhat.com, tglx@linutronix.de,
linux-kernel@vger.kernel.org, joro@8bytes.org,
boris.ostrovsky@oracle.com, jgross@suse.com, x86@kernel.org,
mbenes@suse.com, rostedt@goodmis.org, dvyukov@google.com
Subject: Re: [PATCH v2 03/24] objtool: Handle __sanitize_cov*() tail calls
Date: Fri, 25 Jun 2021 13:05:39 +0200 [thread overview]
Message-ID: <YNW4g8RxydgADtW5@elver.google.com> (raw)
In-Reply-To: <20210624095147.818783799@infradead.org>
On Thu, Jun 24, 2021 at 11:41AM +0200, Peter Zijlstra wrote:
> Turns out the compilers also generate tail calls to __sanitize_cov*(),
> make sure to also patch those out in noinstr code.
>
> Fixes: 0f1441b44e82 ("objtool: Fix noinstr vs KCOV")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Marco Elver <elver@google.com>
Thanks! More reason to not even try to do the same for other
architectures, and the compiler needing the attribute...
> ---
> tools/objtool/arch/x86/decode.c | 20 ++++
> tools/objtool/check.c | 153 ++++++++++++++++++-----------------
> tools/objtool/include/objtool/arch.h | 1
> 3 files changed, 100 insertions(+), 74 deletions(-)
>
> --- a/tools/objtool/arch/x86/decode.c
> +++ b/tools/objtool/arch/x86/decode.c
> @@ -659,6 +659,26 @@ const char *arch_nop_insn(int len)
> return nops[len-1];
> }
>
> +#define BYTE_RET 0xC3
> +
> +const char *arch_ret_insn(int len)
> +{
> + static const char ret[5][5] = {
> + { BYTE_RET },
> + { BYTE_RET, BYTES_NOP1 },
> + { BYTE_RET, BYTES_NOP2 },
> + { BYTE_RET, BYTES_NOP3 },
> + { BYTE_RET, BYTES_NOP4 },
> + };
> +
> + if (len < 1 || len > 5) {
> + WARN("invalid RET size: %d\n", len);
> + return NULL;
> + }
> +
> + return ret[len-1];
> +}
> +
> /* asm/alternative.h ? */
>
> #define ALTINSTR_FLAG_INV (1 << 15)
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -828,6 +828,74 @@ static struct reloc *insn_reloc(struct o
> return insn->reloc;
> }
>
> +static void remove_insn_ops(struct instruction *insn)
> +{
> + struct stack_op *op, *tmp;
> +
> + list_for_each_entry_safe(op, tmp, &insn->stack_ops, list) {
> + list_del(&op->list);
> + free(op);
> + }
> +}
> +
> +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)
> + return;
> +
> + if (insn->call_dest->static_call_tramp) {
> + list_add_tail(&insn->call_node,
> + &file->static_call_list);
> + }
> +
> + if (insn->sec->noinstr &&
> + !strncmp(insn->call_dest->name, "__sanitizer_cov_", 16)) {
> + if (reloc) {
> + reloc->type = R_NONE;
> + elf_write_reloc(file->elf, reloc);
> + }
> +
> + elf_write_insn(file->elf, insn->sec,
> + insn->offset, insn->len,
> + sibling ? arch_ret_insn(insn->len)
> + : arch_nop_insn(insn->len));
> +
> + insn->type = sibling ? INSN_RETURN : INSN_NOP;
> + }
> +
> + if (mcount && !strcmp(insn->call_dest->name, "__fentry__")) {
> + if (sibling)
> + WARN_FUNC("Tail call to __fentry__ !?!?", insn->sec, insn->offset);
> +
> + if (reloc) {
> + reloc->type = R_NONE;
> + elf_write_reloc(file->elf, reloc);
> + }
> +
> + elf_write_insn(file->elf, insn->sec,
> + insn->offset, insn->len,
> + arch_nop_insn(insn->len));
> +
> + insn->type = INSN_NOP;
> +
> + 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);
> +}
> +
> /*
> * Find the destination instructions for all jumps.
> */
> @@ -866,11 +934,7 @@ static int add_jump_destinations(struct
> continue;
> } else if (insn->func) {
> /* internal or external sibling call (with reloc) */
> - insn->call_dest = reloc->sym;
> - if (insn->call_dest->static_call_tramp) {
> - list_add_tail(&insn->call_node,
> - &file->static_call_list);
> - }
> + add_call_dest(file, insn, reloc->sym, true);
> continue;
> } else if (reloc->sym->sec->idx) {
> dest_sec = reloc->sym->sec;
> @@ -926,13 +990,8 @@ static int add_jump_destinations(struct
>
> } else if (insn->jump_dest->func->pfunc != insn->func->pfunc &&
> insn->jump_dest->offset == insn->jump_dest->func->offset) {
> -
> /* internal sibling call (without reloc) */
> - insn->call_dest = insn->jump_dest->func;
> - if (insn->call_dest->static_call_tramp) {
> - list_add_tail(&insn->call_node,
> - &file->static_call_list);
> - }
> + add_call_dest(file, insn, insn->jump_dest->func, true);
> }
> }
> }
> @@ -940,16 +999,6 @@ static int add_jump_destinations(struct
> return 0;
> }
>
> -static void remove_insn_ops(struct instruction *insn)
> -{
> - struct stack_op *op, *tmp;
> -
> - list_for_each_entry_safe(op, tmp, &insn->stack_ops, list) {
> - list_del(&op->list);
> - free(op);
> - }
> -}
> -
> static struct symbol *find_call_destination(struct section *sec, unsigned long offset)
> {
> struct symbol *call_dest;
> @@ -968,6 +1017,7 @@ static int add_call_destinations(struct
> {
> struct instruction *insn;
> unsigned long dest_off;
> + struct symbol *dest;
> struct reloc *reloc;
>
> for_each_insn(file, insn) {
> @@ -977,7 +1027,9 @@ static int add_call_destinations(struct
> reloc = insn_reloc(file, insn);
> if (!reloc) {
> dest_off = arch_jump_destination(insn);
> - insn->call_dest = find_call_destination(insn->sec, dest_off);
> + dest = find_call_destination(insn->sec, dest_off);
> +
> + add_call_dest(file, insn, dest, false);
>
> if (insn->ignore)
> continue;
> @@ -995,9 +1047,8 @@ static int add_call_destinations(struct
>
> } else if (reloc->sym->type == STT_SECTION) {
> dest_off = arch_dest_reloc_offset(reloc->addend);
> - insn->call_dest = find_call_destination(reloc->sym->sec,
> - dest_off);
> - if (!insn->call_dest) {
> + dest = find_call_destination(reloc->sym->sec, dest_off);
> + if (!dest) {
> WARN_FUNC("can't find call dest symbol at %s+0x%lx",
> insn->sec, insn->offset,
> reloc->sym->sec->name,
> @@ -1005,6 +1056,8 @@ static int add_call_destinations(struct
> return -1;
> }
>
> + add_call_dest(file, insn, dest, false);
> +
> } else if (arch_is_retpoline(reloc->sym)) {
> /*
> * Retpoline calls are really dynamic calls in
> @@ -1020,55 +1073,7 @@ static int add_call_destinations(struct
> continue;
>
> } else
> - insn->call_dest = reloc->sym;
> -
> - if (insn->call_dest && insn->call_dest->static_call_tramp) {
> - list_add_tail(&insn->call_node,
> - &file->static_call_list);
> - }
> -
> - /*
> - * Many compilers cannot disable KCOV with a function attribute
> - * so they need a little help, NOP out any KCOV calls from noinstr
> - * text.
> - */
> - if (insn->sec->noinstr &&
> - !strncmp(insn->call_dest->name, "__sanitizer_cov_", 16)) {
> - if (reloc) {
> - reloc->type = R_NONE;
> - elf_write_reloc(file->elf, reloc);
> - }
> -
> - elf_write_insn(file->elf, insn->sec,
> - insn->offset, insn->len,
> - arch_nop_insn(insn->len));
> - insn->type = INSN_NOP;
> - }
> -
> - if (mcount && !strcmp(insn->call_dest->name, "__fentry__")) {
> - if (reloc) {
> - reloc->type = R_NONE;
> - elf_write_reloc(file->elf, reloc);
> - }
> -
> - elf_write_insn(file->elf, insn->sec,
> - insn->offset, insn->len,
> - arch_nop_insn(insn->len));
> -
> - insn->type = INSN_NOP;
> -
> - 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);
> + add_call_dest(file, insn, reloc->sym, false);
> }
>
> return 0;
> --- a/tools/objtool/include/objtool/arch.h
> +++ b/tools/objtool/include/objtool/arch.h
> @@ -82,6 +82,7 @@ unsigned long arch_jump_destination(stru
> unsigned long arch_dest_reloc_offset(int addend);
>
> const char *arch_nop_insn(int len);
> +const char *arch_ret_insn(int len);
>
> int arch_decode_hint_reg(struct instruction *insn, u8 sp_reg);
>
>
>
next prev parent reply other threads:[~2021-06-25 11:05 UTC|newest]
Thread overview: 101+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-24 9:40 [PATCH v2 00/24] objtool/x86: noinstr vs PARAVIRT Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 01/24] x86/xen: Mark cpu_bringup_and_idle() as dead_end_function Peter Zijlstra
2021-06-24 10:49 ` Juergen Gross
2021-06-30 11:47 ` Miroslav Benes
2021-08-20 19:22 ` Josh Poimboeuf
2021-08-20 19:31 ` Josh Poimboeuf
2021-08-23 8:40 ` Juergen Gross
2021-08-30 5:55 ` Juergen Gross
2021-08-30 7:48 ` Peter Zijlstra
2021-08-30 7:56 ` Juergen Gross
2021-09-15 15:49 ` [tip: objtool/core] x86/xen: Move hypercall_page to top of the file tip-bot2 for Josh Poimboeuf
2021-09-15 15:49 ` [tip: objtool/core] x86/xen: Mark cpu_bringup_and_idle() as dead_end_function tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 02/24] objtool: Introduce CFI hash Peter Zijlstra
2021-06-30 12:45 ` Miroslav Benes
2021-07-01 8:35 ` Peter Zijlstra
2021-07-01 8:53 ` Miroslav Benes
2021-08-20 22:27 ` Josh Poimboeuf
2021-08-25 10:13 ` Peter Zijlstra
2021-08-25 12:49 ` Peter Zijlstra
2021-08-25 10:15 ` Peter Zijlstra
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 03/24] objtool: Handle __sanitize_cov*() tail calls Peter Zijlstra
2021-06-25 11:05 ` Marco Elver [this message]
2021-06-28 9:07 ` Peter Zijlstra
2021-06-25 13:38 ` Steven Rostedt
2021-06-26 8:18 ` Peter Zijlstra
2021-06-29 1:07 ` Steven Rostedt
2021-08-20 23:17 ` Josh Poimboeuf
2021-08-25 10:19 ` Peter Zijlstra
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 04/24] x86/kvm: Always inline sev_*guest() Peter Zijlstra
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 05/24] x86/kvm: Always inline vmload() / vmsave() Peter Zijlstra
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 06/24] x86: Always inline context_tracking_guest_enter() Peter Zijlstra
2021-08-20 23:22 ` Josh Poimboeuf
2021-08-20 23:34 ` Josh Poimboeuf
2021-08-25 12:16 ` Peter Zijlstra
[not found] ` <YSZxtBHNJzoUbqdE@ravnborg.org>
2021-08-26 4:10 ` Josh Poimboeuf
2021-08-25 13:38 ` Peter Zijlstra
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 07/24] x86/kvm: Always inline to_svm() Peter Zijlstra
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 08/24] x86/kvm: Always inline evmcs_write64() Peter Zijlstra
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 09/24] x86: Always inline ip_within_syscall_gap() Peter Zijlstra
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 10/24] x86/sev: Fix noinstr for vc_ghcb_invalidate() Peter Zijlstra
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 11/24] locking/lockdep: Avoid RCU-induced noinstr fail Peter Zijlstra
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 12/24] x86/paravirt: Mark arch_local_irq_*() __always_inline Peter Zijlstra
2021-06-24 10:51 ` Juergen Gross
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 13/24] x86/paravirt: Use PVOP_* for paravirt calls Peter Zijlstra
2021-06-24 10:52 ` Juergen Gross
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 14/24] x86/xen: Make read_cr2() noinstr Peter Zijlstra
2021-06-24 10:55 ` Juergen Gross
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58 ` tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 15/24] x86/xen: Make write_cr2() noinstr Peter Zijlstra
2021-06-24 10:56 ` Juergen Gross
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58 ` tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 16/24] x86/xen: Make get_debugreg() noinstr Peter Zijlstra
2021-06-24 10:57 ` Juergen Gross
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58 ` tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 17/24] x86/xen: Make set_debugreg() noinstr Peter Zijlstra
2021-06-24 10:59 ` Juergen Gross
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58 ` tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 18/24] x86/xen: Make save_fl() noinstr Peter Zijlstra
2021-06-24 11:07 ` Juergen Gross
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58 ` tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 19/24] x86/xen: Make hypercall_page noinstr Peter Zijlstra
2021-06-24 11:08 ` Juergen Gross
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58 ` tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 20/24] x86/xen: Make irq_enable() noinstr Peter Zijlstra
2021-06-24 11:09 ` Juergen Gross
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58 ` tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 21/24] x86/xen: Make irq_disable() noinstr Peter Zijlstra
2021-06-24 11:10 ` Juergen Gross
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58 ` tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 22/24] x86/xen: Mark xen_force_evtchn_callback() noinstr Peter Zijlstra
2021-06-24 11:11 ` Juergen Gross
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58 ` tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 23/24] x86/xen: Rework the xen_{cpu,irq,mmu}_ops[] arrays Peter Zijlstra
2021-06-24 12:12 ` Juergen Gross
2021-09-15 15:49 ` [tip: objtool/core] x86/xen: Rework the xen_{cpu,irq,mmu}_opsarrays tip-bot2 for Peter Zijlstra
2021-09-17 12:58 ` tip-bot2 for Peter Zijlstra
2021-06-24 9:41 ` [PATCH v2 24/24] objtool: Support pv_opsindirect calls for noinstr Peter Zijlstra
2021-09-15 15:49 ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58 ` tip-bot2 for Peter Zijlstra
2021-06-24 13:05 ` [PATCH v2 00/24] objtool/x86: noinstr vs PARAVIRT Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YNW4g8RxydgADtW5@elver.google.com \
--to=elver@google.com \
--cc=boris.ostrovsky@oracle.com \
--cc=dvyukov@google.com \
--cc=jgross@suse.com \
--cc=joro@8bytes.org \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mbenes@suse.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).