Re: [PATCH v2 02/10] arm64: asm: Override SYM_FUNC_START when building the kernel with BTI

From: Dave Martin <Dave.Martin@arm•com>
To: Will Deacon <will@kernel•org>
Cc: Catalin Marinas <catalin.marinas@arm•com>,
	Mark Brown <broonie@kernel•org>,
	Vincenzo Frascino <Vincenzo.Frascino@arm•com>,
	Kees Cook <keescook@chromium•org>,
	linux-arm-kernel@lists•infradead.org
Subject: Re: [PATCH v2 02/10] arm64: asm: Override SYM_FUNC_START when building the kernel with BTI
Date: Wed, 6 May 2020 16:33:02 +0100	[thread overview]
Message-ID: <20200506153302.GB21779@arm.com> (raw)
In-Reply-To: <20200506144543.GB12919@willie-the-truck>

On Wed, May 06, 2020 at 03:45:44PM +0100, Will Deacon wrote:
> On Wed, May 06, 2020 at 02:40:21PM +0100, Dave Martin wrote:
> > On Wed, May 06, 2020 at 11:50:06AM +0100, Will Deacon wrote:
> > > On Wed, May 06, 2020 at 11:41:52AM +0100, Mark Brown wrote:
> > > > On Wed, May 06, 2020 at 08:10:26AM +0100, Will Deacon wrote:
> > > > > On Tue, May 05, 2020 at 06:21:00PM +0100, Mark Brown wrote:
> > > > 
> > > > > > > > > aarch64-none-linux-gnu-ld: arch/arm64/kernel/vdso/vgettimeofday.o: warning: BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.
> > > > 
> > > > > > so if this is a clang issue it looks like they fixed it in the clang-10
> > > > > > branch.  I'm not sure it's worth trying to detect and handle this or
> > > > > > not, I don't know how widely deployed toolchains that don't emit the
> > > > > > property are and there's a fairly clear solution.  What do you think?
> > > > 
> > > > > The linker fixes this up when it warns, right? If so, I think the current
> > > > 
> > > > The linker is fixing this up, yes.
> > > > 
> > > > > behaviour is fine *but* we might want to improve the diagnostic a bit not
> > > > > to worry/confuse people. e.g. something like:
> > > > 
> > > > >   "Your compiler is not emitting '.note.gnu.property' sections: forcing
> > > > >    support for BTI in the linker, but consider upgrading your toolchain."
> > > > 
> > > > Well, the theory behind the warning is that if the compiler is emitting
> > > > code suitable for the features described in the note then it should
> > > > always emit the appropriate annotations so the warning is more intended
> > > > to be telling the user that the code is trying to link in code that's
> > > > not built properly and will most likely fail at runtime.  In the current
> > > > situation that's an issue with the toolchain not emitting the
> > > > annotations but the common case expectation is that the issue will be
> > > > that there are object files that weren't built appropriately.
> > > 
> > > Hmm, I suppose, although it's a bit belt-and-braces given that we've got
> > > the right options in KBUILD_CFLAGS. What about:
> > > 
> > > 	"Your compiler is not emitting '.note.gnu.property' sections: forcing
> > > 	 support for BTI in the linker, but check your CFLAGS and consider
> > > 	 upgrading your toolchain."
> > > 
> > > I'd usually not be too bothered, but having run into this yesterday and
> > > not understood the problem, I'd like to save somebody else from puzzling
> > > over this if we can!
> > 
> > I really don't think we should fudge this: if the linker doesn't think
> > the inputs are BTI-enabled then the compiler or linker is broken, or
> > there's a bug in the kernel source tree.
> > 
> > The checking done by the toolchain is important -- if we want to
> > suppress it, we should have an override option than depends on BROKEN
> > (because yes, you're explicitly risking a broken kernel if you do this).
> > 
> > The fact that all gcc and clang both screwed this up in various ways at
> > some point is not our fault, or our problem, providing that fixes are
> > available...
> > 
> > Am I being too paranoid?
> 
> I don't think so, but I'm just looking for an answer to "What do we do if
> people start running into this warning?". As it stands, it sounds like it's
> unlikely that they will, but if they do then we're going to have to hack
> something to make it go away.

If we can do it in a way that makes it obvious that we consider it
unsafe, I guess having an override would be OK.  I feel the user should
be required to do something explicit like enable a dangerous-looking
Kconfig option, rather than us helpfully working around it
automatically.

Unless someone can explain why this is not dangerous :)

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists•infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel