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

Dave Martin
2020-05-06T15:33:10Z

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