linux-kernel - Re: x86 entry perf unwinding failure (missing IRET

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: <20200428164444.GD16910@hirez.programming.kicks-ass.net>
Date:   Tue, 28 Apr 2020 18:44:44 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     Jann Horn <jannh@...gle.com>, Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        the arch/x86 maintainers <x86@...nel.org>,
        kernel list <linux-kernel@...r.kernel.org>,
        alexandre.chartre@...cle.com
Subject: Re: x86 entry perf unwinding failure (missing IRET_REGS annotation
 on stack switch?)

On Tue, Apr 28, 2020 at 10:49:09AM -0500, Josh Poimboeuf wrote:
> > @@ -2439,12 +2445,6 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
> >  
> >  	sec = insn->sec;
> >  
> > -	if (insn->alt_group && list_empty(&insn->alts)) {
> > -		WARN_FUNC("don't know how to handle branch to middle of alternative instruction group",
> > -			  sec, insn->offset);
> > -		return 1;
> > -	}
> > -
> 
> ACK (separate patch)
> 
> >  	while (1) {
> >  		next_insn = next_insn_same_sec(file, insn);
> >  

Yeah, there is one from Julien that does this:

  20200327152847.15294-6-jthierry@...hat.com

> > @@ -2494,8 +2494,16 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
> >  				}
> >  			}
> >  
> > -			if (skip_orig)
> > +			if (skip_orig) {
> > +				struct instruction *prev_insn = insn;
> > +				sec_for_each_insn_continue(file, insn) {
> > +					if (!insn->alt_group)
> > +						break;
> > +					if (!insn->visited)
> > +						insn->cfi = prev_insn->cfi;
> > +				}
> >  				return 0;
> > +			}
> 
> NACK :-)
> 
> What happens if you have two alternatives adjacent to each other (which
> can definitely happen in this scenario)?

Alexandre's alt_group would help:

  20200414103618.12657-3-alexandre.chartre@...cle.com

Then we can do something like:

static void fill_alternative(struct instruction *insn)
{
	struct instruction *first_insn = insn;
	int alt_group = insn->alt_group;

	sec_for_each_insn_continue(file, insn) {
		if (insn->alt_group != alt_group)
			break;
		if (!insn->visited)
			insn->cfi = first_insn->cfi;
	}
}

> I still like my patch, at least the hack is done before the validate
> code, so validate_branch() itself is simpler.

But it doesn't handle the case where the alternatives themselves have
unreachable holes in them, if that happens we'll generate spurious ORC
entries for them.