Re: [PATCH] arch: x86: power: cpu: init %gs before __restore_processor_state (clang)

From: Nick Desaulniers <ndesaulniers@google.com>
To: Pavel Machek <pavel@denx.de>
Cc: Roman Kiryanov <rkir@google.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<x86@kernel.org>,
	linux-pm@vger.kernel.org, Greg KH <gregkh@linuxfoundation.org>,
	Alistair Delva <adelva@google.com>,
	Haitao Shan <hshan@google.com>
Subject: Re: [PATCH] arch: x86: power: cpu: init %gs before __restore_processor_state (clang)
Date: Tue, 15 Sep 2020 15:21:25 -0700	[thread overview]
Message-ID: <CAKwvOdnuR9GF2tZuyS7Fe8i-S0W2YZ45qP8xJRm_gf+g+oF4=Q@mail.gmail.com> (raw)
In-Reply-To: <20200915221708.GA26453@duo.ucw.cz>

On Tue, Sep 15, 2020 at 3:17 PM Pavel Machek <pavel@denx.de> wrote:
>
> Hi!
>
> > From: Haitao Shan <hshan@google.com>
> >
> > This is a workaround which fixes triple fault
> > in __restore_processor_state on clang when
> > built with LTO.
> >
> > When load_TR_desc and load_mm_ldt are inlined into
> > fix_processor_context due to LTO, they cause
> > fix_processor_context (or in this case __restore_processor_state,
> > as fix_processor_context was inlined into __restore_processor_state)
> > to access the stack canary through %gs, but before
> > __restore_processor_state has restored the previous value
> > of %gs properly. LLVM appears to be inlining functions with stack
> > protectors into functions compiled with -fno-stack-protector,
> > which is likely a bug in LLVM's inliner that needs to be fixed.
>
> That's rather ugly.
>
> Would it be easier to simply mark those functions as noinline or
> something like that?

It is possible, and a possible solution here.  We discussed that
internally, and found it to not be great.  You only want to prevent
inlining across TUs for LTO; currently there's no great way to express
that in source code.  As in "go ahead and inline this from calls
within the TU that this is defined, but don't inline across TUs other
than from where the callee is defined."  I think we should create a
function level attribute for expressing that intention to the
compiler.  You could emulate that somewhat by wrapping the noinline
attribute in CONFIG_LTO_CLANG, but that doesn't solve allowing
inlining within the same TU.  If you've been following the thread,
there's multiple ways to skin this cat; your suggestion is one we did
consider, just not on the public mailing list.
-- 
Thanks,
~Nick Desaulniers