linux-kernel - Re: [PATCH 3/7] objtool: Add support for intra-function calls

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, 2 Apr 2020 10:54:36 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Alexandre Chartre <alexandre.chartre@...cle.com>,
        Julien Thierry <jthierry@...hat.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org, tglx@...utronix.de
Subject: Re: [PATCH 3/7] objtool: Add support for intra-function calls

On Thu, Apr 02, 2020 at 05:04:07PM +0200, Peter Zijlstra wrote:
> On Thu, Apr 02, 2020 at 03:24:45PM +0200, Alexandre Chartre wrote:
> > On 4/2/20 2:53 PM, Julien Thierry wrote:
> > > On 4/2/20 9:22 AM, Alexandre Chartre wrote:
> 
> > > > +Â Â Â  sec = find_section_by_name(file->elf,
> > > > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  ".rela.discard.intra_function_call");
> > > 
> > > I'm wondering, do we really need to annotate the intra_function_call
> > > and group the in a section?
> > > 
> > > Would it be a problem to consider all (static) call instructions with
> > > a destination that is not the start offset of a symbol to be an
> > > intra-function call (and set insn->intra_function_call and
> > > insn->jump_dest accordingly)?
> > 
> > Correct, we could automatically detect intra-function calls instead of
> > having to annotate them. However, I choose to annotate them because I don't
> > think that's not an expected construct in a "normal" code flow (at least
> > on x86). So objtool would still issue a warning on intra-function calls
> > by default, and you can annotate them to indicate if they are expected.
> 
> I wondered the same thing when reading the patch. I'm confliected on
> this. On the one hand auto-detecting this seems like an excellent idea.
> 
> If/when the compiler generates them, they had better be okay too.
> 
> Josh?

In general I prefer to keep it simple, and keep the annotations to a
minimum.  And I don't think this warning has ever found anything useful.
So I'd be inclined to say just allow them and automatically detect them.

However the fact that arm64 asm actually uses them worries me a bit.

So for me it kind of hinges on whether arm64 has a legitimate use case
for them, or if the warning actually points to smelly code.

-- 
Josh