Re: [PATCH 4/6] objtool: Replace STACK_FRAME_NON_STANDARD annotation

From: Andy Lutomirski <luto@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Julien Thierry <julien.thierry@arm.com>,
	Will Deacon <will.deacon@arm.com>, Ingo Molnar <mingo@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	James Morse <james.morse@arm.com>,
	valentin.schneider@arm.com, Brian Gerst <brgerst@gmail.com>,
	Andrew Lutomirski <luto@kernel.org>,
	Borislav Petkov <bp@alien8.de>,
	Denys Vlasenko <dvlasenk@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4/6] objtool: Replace STACK_FRAME_NON_STANDARD annotation
Date: Wed, 27 Feb 2019 16:30:08 -0800	[thread overview]
Message-ID: <CALCETrUHyUA=1BOK=pL1O6M2B4E9JZv7F3eP7rHbeEysvgZPEQ@mail.gmail.com> (raw)
In-Reply-To: <20190227122059.GO32494@hirez.programming.kicks-ass.net>

On Wed, Feb 27, 2019 at 4:21 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Feb 25, 2019 at 10:11:24AM -0600, Josh Poimboeuf wrote:
> > On Mon, Feb 25, 2019 at 01:43:34PM +0100, Peter Zijlstra wrote:
>
> > > -#define STACK_FRAME_NON_STANDARD(func) \
> > > -   static void __used __section(.discard.func_stack_frame_non_standard) \
> > > -           *__func_stack_frame_non_standard_##func = func
> > > +#define STACK_FRAME_NON_STANDARD(func)                                     \
> > > +   asm (".pushsection .discard.nonstd_frame_strtab, \"S\", @3\n\t" \
> > > +        "999: .ascii \"" #func "\"\n\t"                            \
> > > +        "     .byte 0\n\t"                                         \
> > > +        ".popsection\n\t"                                          \
> > > +        ".pushsection .discard.nonstd_frame\n\t"                   \
> > > +        ".long 999b - .\n\t"                                       \
> > > +        ".popsection\n\t")
> > > +
> >
> > I don't think this will work in the case where GCC does an IPA
> > optimization and renames the function (e.g., renames func to
> > func.isra.1234), right?  That might be a deal breaker...
>
> Or; as has been found by 0day; the whole function gets inlined and
> the symbol no longer exists at all.
>
> That's curable with a noinline, but all things considered, I think we
> should go back to the old horrible scheme. Andy?

Ugh, I guess.  I'm wondering just how atrocious the generated code is.

Just as a thought experiment, here are some other options:

1. Make a tiny GCC plugin that parses a special attribute on function
declarations and emits a record that describes that attribute into the
object file for each referenced symbol that comes from that
declaration.  (I know nothing about GCC internals, so I don't know how
hard this would be.)

2. Fiddle with the function names.  Turn a function called foo() into
__uaccess_safe_foo() and also emit a weak alias from that to foo.
This is probably every bit as bad as taking the address.

3. Take advantage of the fact that only static functions are (for now)
subject to this IPA stuff.  So take the address of a static function
or just declare that calling a static function is uaccess safe.

--Andy