Re: [PATCH] x86/tools/relocs: Add _etext and __end_of_kernel_reserve to S_REL

[Arvind Sankar <nivedita@alum.mit.edu>]

On Mon, Jan 13, 2020 at 02:43:06PM +0100, Borislav Petkov wrote:
> On Sat, Jan 11, 2020 at 12:20:48PM -0500, Arvind Sankar wrote:
> > I'm not sure if that's the same issue. The root cause for the one I
> > reported is described in more detail in [1], and the change that makes
> > these symbols no longer absolute is commit d2667025dd30 in binutils-gdb
> > (sourceware.org seems to be taking too long to respond from here so I
> > don't have the web link).
> 
> My binutils guy says that the proper fix should be to make those two
> symbols section-relative, i.e., move _etext at the end of the .text
> section and so on.
> 
> Please check whether this fixes the build issue too because if it does,
> it would be The RightThing(tm).

According to Kees in [1] commit 392bef709659 ("x86/build: Move _etext to
actual end of .text"), keeping _etext inside the .text section was
incorrect in some situations with clang. That commit was later reverted
because it broke old toolchains, and the new commit that did the same
thing commit b907693883fd ("x86/vmlinux: Actually use _etext for the end
of the text segment") does not mention that rationale.

Kees, is it still required to keep _etext out of .text, or can it be
moved back in? If it has to remain outside, 
	_etext = _stext + SIZEOF(.text);
seems to leave _etext as a relative symbol, even though from the
binutils documentation I'd have expected SIZEOF(.text), as a number, to
be treated as an absolute expression outside an output section. Could
you check if that would work for your case?

The __end_of_kernel_reserve can be made relative by just assigning it
__bss_stop instead of the location counter.

I will note that the purpose of S_REL in relocs.c was originally to
handle exactly this case of symbols defined outside output sections:

/*
 * These symbols are known to be relative, even if the linker marks them
 * as absolute (typically defined outside any section in the linker script.)
 */
        [S_REL] =

> 
> > I'm running gentoo, but building the kernel using binutils-2.21.1
> > compiled from the GNU source tarball, and gcc-4.6.4 again compiled from
> > source. (It's not something I normally need but I was investigating
> > something else to see what exactly happens with older toolchains.)
> > 
> > I used the below to compile the kernel (I added in
> > readelf/objdump/objcopy just now, and it does build until the relocs
> > error). The config is x86-64 defconfig with CONFIG_RETPOLINE overridden
> > to n (since gcc 4.6.4 doesn't support retpoline).
> > 
> > make O=~/kernel64 -j LD=~/old/bin/ld AS=~/old/bin/as READELF=~/old/bin/readelf \
> > 	OBJDUMP=~/old/bin/objdump OBJCOPY=~/old/bin/objcopy GCC=~/old/bin/gcc
> 
> Make this all part of your commit message because it explains in detail
> how exactly you've triggered it so that anyone else reading this can
> reproduce her/himself.

How to reproduce is just "build with old binutils". I don't see it's
reasonable to include a tutorial on how to build the kernel with a
toolchain that's not installed in the default PATH, as part of the commit
message.

> 
> Thx.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

[1] x86/build: Move _etext to actual end of .text

When building x86 with Clang LTO and CFI, CFI jump regions are
automatically added to the end of the .text section late in linking. As a
result, the _etext position was being labelled before the appended jump
regions, causing confusion about where the boundaries of the executable
region actually are in the running kernel, and broke at least the fault
injection code. This moves the _etext mark to outside (and immediately
after) the .text area, as it already the case on other architectures
(e.g. arm64, arm).