Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Thu, 14 Apr 2022 20:49:48 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Josh Poimboeuf <jpoimboe@redhat.com>
Cc:     x86@kernel.org, linux-kernel@vger.kernel.org,
        Miroslav Benes <mbenes@suse.cz>
Subject: Re: [PATCH 10/18] objtool: Extricate ibt from stack validation
Message-ID: <YlhszNnqAnGHGmak@hirez.programming.kicks-ass.net>
References: <cover.1649891421.git.jpoimboe@redhat.com>
 <44a73f724b51c4a994edc43536b7a7ee5e972b40.1649891421.git.jpoimboe@redhat.com>
 <YlfS7twQVCHGgtCV@hirez.programming.kicks-ass.net>
 <20220414154449.5moa7xsczwybbqhd@treble>
 <YlhN+GFZlycwydSv@hirez.programming.kicks-ass.net>
 <20220414170550.v2jmdfhmz7zbuug6@treble>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220414170550.v2jmdfhmz7zbuug6@treble>
Precedence: bulk

On Thu, Apr 14, 2022 at 10:05:50AM -0700, Josh Poimboeuf wrote:
> On Thu, Apr 14, 2022 at 06:38:16PM +0200, Peter Zijlstra wrote:
> > On Thu, Apr 14, 2022 at 08:44:49AM -0700, Josh Poimboeuf wrote:
> > 
> > > Ok.  That was subtle, it needs a comment or two.  I had the distinct
> > > feeling I was introducing a bug, then I got distracted ;-)
> > 
> > Right, lemme try and not forget to write one ;-)
> 
> I'm rewriting the code anyway, I'll add some comments.
> 
> > > Doesn't the compiler give those special cases ENDBR anyway?  Just
> > > wondering why we avoid the warning for those.
> > 
> > Sure, but this is about not scribbling that ENDBR with a NOP.
> 
> Right, but it only prints warnings for data sections, despite marking
> others:
> 
> -                       dest = validate_ibt_reloc(file, reloc);
> -                       if (is_data && dest && !dest->noendbr)
> -                               warn_noendbr("data ", sec, reloc->offset, dest);
> 

Right, so validate_ibt_reloc() does two things:

 - it removes the instruction from the seal list (if it was still on it)
 - it returns the dest instruction if it isn't ENDBR (or a static call
   trampoline, which we know will never be indirectly called).

So that first thing is always important; we should not seal too many
things (.initcall and .ksymtab would be bad etc..), that second thing is
only useful if there is potential control flow.

That is, .ksymtab is not used for control flow (directly), it's the
symbol table for the module linker. OTOH .initcall does have control
flow, but it is 100% control flow.

In neither case does it really make sense to warn (then again, they also
should not trigger warns I suppose).

.data/.rodata otoh is more questionable, typically a code reference
there is a function pointer and we really want an ENDBR there, but it's
not always the case, hence the warns from there.

Now, I have some vague memories of getting a lot of noise from some
other sections, but I can't really remember now :-/