linux-kernel - Re: [v8,02/12] objtool: Allow alternatives to be ignored

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]
Date:   Thu, 18 Jan 2018 13:33:15 -0600
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     "Woodhouse, David" <dwmw@...zon.co.uk>,
        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
Subject: Re: [v8,02/12] objtool: Allow alternatives to be ignored

On Thu, Jan 18, 2018 at 11:09:31AM -0800, Guenter Roeck wrote:
> 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 ?

Not sure, does your gcc have retpolines?  Give me your .o file and I can
diagnose it.

I intended to have an error msg instead of a seg fault for this
situation, just haven't had a chance to improve that yet in the midst of
all the hoopla.

-- 
Josh