| 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
| ||
|
Message-ID: <20180118190931.GA27143@roeck-us.net>
Date: Thu, 18 Jan 2018 11:09:31 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: "Woodhouse, David" <dwmw@...zon.co.uk>
Cc: Andi Kleen <ak@...ux.intel.com>, Paul Turner <pjt@...gle.com>,
LKML <linux-kernel@...r.kernel.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Greg Kroah-Hartman <gregkh@...ux-foundation.org>,
Tim Chen <tim.c.chen@...ux.intel.com>,
Dave Hansen <dave.hansen@...el.com>, tglx@...utronix.de,
Kees Cook <keescook@...gle.com>,
Rik van Riel <riel@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Andy Lutomirski <luto@...capital.net>,
Jiri Kosina <jikos@...nel.org>, gnomes@...rguk.ukuu.org.uk,
x86@...nel.org, thomas.lendacky@....com,
Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: [v8,02/12] objtool: Allow alternatives to be ignored
Hi folks,
On Thu, Jan 11, 2018 at 09:46:24PM +0000, Woodhouse, David wrote:
> Getting objtool to understand retpolines is going to be a bit of a
> challenge. For now, take advantage of the fact that retpolines are
> patched in with alternatives. Just read the original (sane)
> non-alternative instruction, and ignore the patched-in retpoline.
>
> This allows objtool to understand the control flow *around* the
> retpoline, even if it can't yet follow what's inside. This means the
> ORC unwinder will fail to unwind from inside a retpoline, but will work
> fine otherwise.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
> Signed-off-by: David Woodhouse <dwmw@...zon.co.uk>
> ---
> tools/objtool/check.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++-----
> tools/objtool/check.h | 2 +-
> 2 files changed, 57 insertions(+), 7 deletions(-)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index de053fb..f40d46e 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -428,6 +428,40 @@ static void add_ignores(struct objtool_file *file)
> }
>
> /*
> + * FIXME: For now, just ignore any alternatives which add retpolines. This is
> + * a temporary hack, as it doesn't allow ORC to unwind from inside a retpoline.
> + * But it at least allows objtool to understand the control flow *around* the
> + * retpoline.
> + */
> +static int add_nospec_ignores(struct objtool_file *file)
> +{
> + struct section *sec;
> + struct rela *rela;
> + struct instruction *insn;
> +
> + sec = find_section_by_name(file->elf, ".rela.discard.nospec");
> + if (!sec)
> + return 0;
> +
> + list_for_each_entry(rela, &sec->rela_list, list) {
> + if (rela->sym->type != STT_SECTION) {
> + WARN("unexpected relocation symbol type in %s", sec->name);
> + return -1;
> + }
> +
> + insn = find_insn(file, rela->sym->sec, rela->addend);
> + if (!insn) {
> + WARN("bad .discard.nospec entry");
> + return -1;
> + }
> +
> + insn->ignore_alts = true;
> + }
> +
> + return 0;
> +}
> +
> +/*
> * Find the destination instructions for all jumps.
> */
> static int add_jump_destinations(struct objtool_file *file)
> @@ -509,11 +543,18 @@ static int add_call_destinations(struct objtool_file *file)
> dest_off = insn->offset + insn->len + insn->immediate;
> insn->call_dest = find_symbol_by_offset(insn->sec,
> dest_off);
> + /*
> + * FIXME: Thanks to retpolines, it's now considered
> + * normal for a function to call within itself. So
> + * disable this warning for now.
> + */
> +#if 0
> if (!insn->call_dest) {
> WARN_FUNC("can't find call dest symbol at offset 0x%lx",
> insn->sec, insn->offset, dest_off);
> return -1;
> }
> +#endif
This crashes for me in is_fentry_call().
Program received signal SIGSEGV, Segmentation fault.
is_fentry_call (insn=<optimized out>, insn=<optimized out>) at check.c:1113
1113 if (insn->type == INSN_CALL &&
(gdb) info stack
#0 is_fentry_call (insn=<optimized out>, insn=<optimized out>) at check.c:1113
#1 validate_branch (file=0x7ffffff7e440, first=0x7ffffff7e128, state=...) at check.c:1747
#2 0x0000000000404bd3 in validate_branch (file=0x7ffffff7e440, first=0x7ffffff7e128, state=...) at check.c:1770
#3 0x0000000000406783 in validate_functions (file=<optimized out>) at check.c:1933
#4 check (_objname=0x6bb9d0 "", _no_fp=40, no_unreachable=4, orc=false) at check.c:2006
#5 0x00000000004021c1 in handle_internal_command (argv=0x7fffffffe5c0, argc=4) at objtool.c:108
#6 main (argc=4, argv=0x7fffffffe5c0) at objtool.c:131
This is not entirely surprising, since insn->call_dest is NULL and
is_fentry_call() doesn't expect that.
How is this supposed to work ? What am I missing ?
Guenter
> } else if (rela->sym->type == STT_SECTION) {
> insn->call_dest = find_symbol_by_offset(rela->sym->sec,
> rela->addend+4);
> @@ -678,12 +719,6 @@ static int add_special_section_alts(struct objtool_file *file)
> return ret;
>
> list_for_each_entry_safe(special_alt, tmp, &special_alts, list) {
> - alt = malloc(sizeof(*alt));
> - if (!alt) {
> - WARN("malloc failed");
> - ret = -1;
> - goto out;
> - }
>
> orig_insn = find_insn(file, special_alt->orig_sec,
> special_alt->orig_off);
> @@ -694,6 +729,10 @@ static int add_special_section_alts(struct objtool_file *file)
> goto out;
> }
>
> + /* Ignore retpoline alternatives. */
> + if (orig_insn->ignore_alts)
> + continue;
> +
> new_insn = NULL;
> if (!special_alt->group || special_alt->new_len) {
> new_insn = find_insn(file, special_alt->new_sec,
> @@ -719,6 +758,13 @@ static int add_special_section_alts(struct objtool_file *file)
> goto out;
> }
>
> + alt = malloc(sizeof(*alt));
> + if (!alt) {
> + WARN("malloc failed");
> + ret = -1;
> + goto out;
> + }
> +
> alt->insn = new_insn;
> list_add_tail(&alt->list, &orig_insn->alts);
>
> @@ -1035,6 +1081,10 @@ static int decode_sections(struct objtool_file *file)
>
> add_ignores(file);
>
> + ret = add_nospec_ignores(file);
> + if (ret)
> + return ret;
> +
> ret = add_jump_destinations(file);
> if (ret)
> return ret;
> diff --git a/tools/objtool/check.h b/tools/objtool/check.h
> index 47d9ea7..dbadb30 100644
> --- a/tools/objtool/check.h
> +++ b/tools/objtool/check.h
> @@ -44,7 +44,7 @@ struct instruction {
> unsigned int len;
> unsigned char type;
> unsigned long immediate;
> - bool alt_group, visited, dead_end, ignore, hint, save, restore;
> + bool alt_group, visited, dead_end, ignore, hint, save, restore, ignore_alts;
> struct symbol *call_dest;
> struct instruction *jump_dest;
> struct list_head alts;
Powered by blists - more mailing lists