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

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

[Arvind Sankar]

Pre-2.23 binutils makes symbols defined outside sections absolute, so
these two symbols break the build on old linkers.

Fixes: b907693883fd ("x86/vmlinux: Actually use _etext for the end of the text segment")
Fixes: c603a309cc75 ("x86/mm: Identify the end of the kernel area to be reserved")
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/tools/relocs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index ce7188cbdae5..0a6146d6414f 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -78,6 +78,8 @@ static const char * const sym_regex_kernel[S_NSYMTYPES] = {
 	"__end_rodata_hpage_align|"
 #endif
 	"__vvar_page|"
+	"_etext|"
+	"__end_of_kernel_reserve|"
 	"_end)$"
 };
 
-- 
2.24.1

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

[Borislav Petkov]

On Fri, Jan 10, 2020 at 03:23:49PM -0500, Arvind Sankar wrote:
> Pre-2.23 binutils makes symbols defined outside sections absolute, so
> these two symbols break the build on old linkers.

-ENOTENOUGHINFO

Which old linkers, how exactly do they break the build, etc etc?

Please give exact reproduction steps.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Arvind Sankar]

On Fri, Jan 10, 2020 at 09:38:28PM +0100, Borislav Petkov wrote:
> On Fri, Jan 10, 2020 at 03:23:49PM -0500, Arvind Sankar wrote:
> > Pre-2.23 binutils makes symbols defined outside sections absolute, so
> > these two symbols break the build on old linkers.
> 
> -ENOTENOUGHINFO
> 
> Which old linkers, how exactly do they break the build, etc etc?
> 
> Please give exact reproduction steps.
> 
> Thx.
> 

binutils-2.21 and -2.22. An x86-64 defconfig will fail with
	Invalid absolute R_X86_64_32S relocation: _etext
and after fixing that one, with
	Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve

Similar errors with 32-bit defconfig.

Should I resend with that added to the commit message?

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

[Kees Cook]

On Fri, Jan 10, 2020 at 09:38:28PM +0100, Borislav Petkov wrote:
> On Fri, Jan 10, 2020 at 03:23:49PM -0500, Arvind Sankar wrote:
> > Pre-2.23 binutils makes symbols defined outside sections absolute, so
> > these two symbols break the build on old linkers.
> 
> -ENOTENOUGHINFO
> 
> Which old linkers, how exactly do they break the build, etc etc?
> 
> Please give exact reproduction steps.

Mauro (now CCed) ran into this too, but on 32-bit builds only with older
binutils. I hadn't set up an environment to try to reproduce it yet, but
it seems like this patch would fix it. Mauro can you test this? Does it
fix it for you too?

https://lore.kernel.org/lkml/20200110202349.1881840-1-nivedita@alum.mit.edu/

-Kees

-- 
Kees Cook

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

[Arvind Sankar]

Prior to binutiles-2.23, ld treats the location counter as absolute if
used outside an output section definition. From version 2.23 onwards,
the location counter is treated as relative to an adjacent output
section (usually the previous one, unless there isn't one or the
location counter has been assigned to previously, in which case the next
one).

The result is that a symbol definition in the linker script, such as
	_etext = .;
that appears outside an output section definition makes _etext an
absolute symbol prior to binutils-2.23 and a relative symbol from
version 2.23 onwards. So when using a 2.21 or 2.22 vintage linker, the
build fails with
	Invalid absolute R_X86_64_32S relocation: _etext
for x86-64, and a similar message with R_386_32 for x86-32.

Fix this by adding these symbols to S_REL to tell the relocs tool that
these should always be treated as relative symbols needing relocation.

Fixes: b907693883fd ("x86/vmlinux: Actually use _etext for the end of the text segment")
Fixes: c603a309cc75 ("x86/mm: Identify the end of the kernel area to be reserved")
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
v2: Added a more detailed commit message

 arch/x86/tools/relocs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index ce7188cbdae5..0a6146d6414f 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -78,6 +78,8 @@ static const char * const sym_regex_kernel[S_NSYMTYPES] = {
 	"__end_rodata_hpage_align|"
 #endif
 	"__vvar_page|"
+	"_etext|"
+	"__end_of_kernel_reserve|"
 	"_end)$"
 };
 
-- 
2.24.1

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

[Arvind Sankar]

On Fri, Jan 10, 2020 at 04:50:05PM -0500, Arvind Sankar wrote:
> Prior to binutiles-2.23, ld treats the location counter as absolute if
                  ^ Please fix this typo before merging (thanks)

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

[Borislav Petkov]

On Fri, Jan 10, 2020 at 03:50:29PM -0500, Arvind Sankar wrote:
> On Fri, Jan 10, 2020 at 09:38:28PM +0100, Borislav Petkov wrote:
> > On Fri, Jan 10, 2020 at 03:23:49PM -0500, Arvind Sankar wrote:
> > > Pre-2.23 binutils makes symbols defined outside sections absolute, so
> > > these two symbols break the build on old linkers.
> > 
> > -ENOTENOUGHINFO
> > 
> > Which old linkers, how exactly do they break the build, etc etc?
> > 
> > Please give exact reproduction steps.
> > 
> > Thx.
> > 
> 
> binutils-2.21 and -2.22. An x86-64 defconfig will fail with
> 	Invalid absolute R_X86_64_32S relocation: _etext
> and after fixing that one, with
> 	Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve

I'm still not clear as to why this happens. I tried reproducing on
openSUSE 12.1 which has

Repository: openSUSE-12.1-Oss
Name: binutils
Version: 2.21.1-12.1.4

and the build there fails with:

objdump: arch/x86/lib/clear_page_64.o: File format not recognized
objdump: arch/x86/lib/cmdline.o: File format not recognized
objdump: arch/x86/lib/cmpxchg16b_emu.o: File format not recognized
objdump: arch/x86/lib/copy_page_64.o: File format not recognized
objdump: arch/x86/lib/copy_user_64.o: File format not recognized
objdump: arch/x86/lib/cpu.o: File format not recognized
...

and objdump is part of binutils.

Now, this looks like another symptom of what you're reporting but what
we're missing is the rootcause about *why* this happens.

Because if the issue is hard to fix or similar, then we probably should
raise the minimum supported binutils version from 2.21 to something
newer and not do this fix.

But before we do that, we need a proper analysis as to why it happens.

Also, what distro are you using to reproduce it on?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Arvind Sankar]

On Sat, Jan 11, 2020 at 02:02:43PM +0100, Borislav Petkov wrote:
> On Fri, Jan 10, 2020 at 03:50:29PM -0500, Arvind Sankar wrote:
> > On Fri, Jan 10, 2020 at 09:38:28PM +0100, Borislav Petkov wrote:
> > > On Fri, Jan 10, 2020 at 03:23:49PM -0500, Arvind Sankar wrote:
> > > > Pre-2.23 binutils makes symbols defined outside sections absolute, so
> > > > these two symbols break the build on old linkers.
> > > 
> > > -ENOTENOUGHINFO
> > > 
> > > Which old linkers, how exactly do they break the build, etc etc?
> > > 
> > > Please give exact reproduction steps.
> > > 
> > > Thx.
> > > 
> > 
> > binutils-2.21 and -2.22. An x86-64 defconfig will fail with
> > 	Invalid absolute R_X86_64_32S relocation: _etext
> > and after fixing that one, with
> > 	Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve
> 
> I'm still not clear as to why this happens. I tried reproducing on
> openSUSE 12.1 which has
> 
> Repository: openSUSE-12.1-Oss
> Name: binutils
> Version: 2.21.1-12.1.4
> 
> and the build there fails with:
> 
> objdump: arch/x86/lib/clear_page_64.o: File format not recognized
> objdump: arch/x86/lib/cmdline.o: File format not recognized
> objdump: arch/x86/lib/cmpxchg16b_emu.o: File format not recognized
> objdump: arch/x86/lib/copy_page_64.o: File format not recognized
> objdump: arch/x86/lib/copy_user_64.o: File format not recognized
> objdump: arch/x86/lib/cpu.o: File format not recognized
> ...
> 
> and objdump is part of binutils.
> 
> Now, this looks like another symptom of what you're reporting but what
> we're missing is the rootcause about *why* this happens.
> 
> Because if the issue is hard to fix or similar, then we probably should
> raise the minimum supported binutils version from 2.21 to something
> newer and not do this fix.
> 
> But before we do that, we need a proper analysis as to why it happens.
> 
> Also, what distro are you using to reproduce it on?
> 
> Thx.
> 

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).

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

[1] https://lore.kernel.org/lkml/20200110215005.2164353-1-nivedita@alum.mit.edu/

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

[Arvind Sankar]

On Sat, Jan 11, 2020 at 12:20:48PM -0500, Arvind Sankar wrote:
> 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
							    ^^ that should be CC
> 

The built kernel (with this patch) does boot till userspace on qemu, so
these old versions aren't producing a completely broken kernel.

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

[Mauro Rossi]

Message resent as plain text to linux-kernel@vger.kernel.org
because of rejection of previous message

---------- Forwarded message ---------
From: Mauro Rossi <issor.oruam@gmail.com>
Date: Sat, Jan 11, 2020 at 8:36 PM
Subject: Re: [PATCH] x86/tools/relocs: Add _etext and
__end_of_kernel_reserve to S_REL
To: Kees Cook <keescook@chromium.org>
Cc: Borislav Petkov <bp@alien8.de>, Arvind Sankar
<nivedita@alum.mit.edu>, Thomas Gleixner <tglx@linutronix.de>, Ingo
Molnar <mingo@redhat.com>, H. Peter Anvin <hpa@zytor.com>,
<x86@kernel.org>, Linux Kernel Mailing List
<linux-kernel@vger.kernel.org>, Thomas Lendacky
<Thomas.Lendacky@amd.com>


Hello Kees,

On Fri, Jan 10, 2020 at 9:56 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Jan 10, 2020 at 09:38:28PM +0100, Borislav Petkov wrote:
> > On Fri, Jan 10, 2020 at 03:23:49PM -0500, Arvind Sankar wrote:
> > > Pre-2.23 binutils makes symbols defined outside sections absolute, so
> > > these two symbols break the build on old linkers.
> >
> > -ENOTENOUGHINFO
> >
> > Which old linkers, how exactly do they break the build, etc etc?
> >
> > Please give exact reproduction steps.
>
> Mauro (now CCed) ran into this too, but on 32-bit builds only with older
> binutils. I hadn't set up an environment to try to reproduce it yet, but
> it seems like this patch would fix it. Mauro can you test this? Does it
> fix it for you too?
>
> https://lore.kernel.org/lkml/20200110202349.1881840-1-nivedita@alum.mit.edu/
>
> -Kees


The patch solves the issue with Android build of 32bit kernel,
I don't see 'Invalid absolute R_386_32 relocation' errors anymore.

Thanks a lot!

Mauro

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

[Borislav Petkov]

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).

> 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.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Arvind Sankar]

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).

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

[Borislav Petkov]

On Mon, Jan 13, 2020 at 11:13:10AM -0500, Arvind Sankar wrote:
> 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:

And we should try not to do hacks, if it can be fixed properly, as
binutils expects symbols to be usually relative to a section.

> 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.

The point is that it should be clear that it should state whether it is
something you trigger with some stock distro which has been shipping
this way or it is something you've customly created. Huge difference.

So pls make sure that is clear from the commit message.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Arvind Sankar]

On Mon, Jan 13, 2020 at 05:38:55PM +0100, Borislav Petkov wrote:
> On Mon, Jan 13, 2020 at 11:13:10AM -0500, Arvind Sankar wrote:
> > 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.
> 
> The point is that it should be clear that it should state whether it is
> something you trigger with some stock distro which has been shipping
> this way or it is something you've customly created. Huge difference.
> 
> So pls make sure that is clear from the commit message.
> 

How is "breaks with binutils before version 2.23" not clear enough? It
will break regardless of whether distro shipped v2.21 or you built
v2.21. I'm _not_ creating a custom binutils with my own patches
specifically to trigger this issue, it's stock binutils, as stock as you
can get it.

Do you really want me to say in the commit message "to reproduce, first
compile binutils-2.21 from source, then try to build the kernel with
it"? Including this information would make sense only if the problem
wasn't with stock binutils, but only with some specific distro's patched
version. _Then_ it would make sense to say something like "binutils
package v-xxx shipped with OpenSUSE v-yyy was broken, this commit works
around it". This is a problem with _any_ binutils-2.21, there's nothing
special about how you need to build it.

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

[Borislav Petkov]

On Mon, Jan 13, 2020 at 12:59:38PM -0500, Arvind Sankar wrote:
> How is "breaks with binutils before version 2.23" not clear enough? It

Nevermind, I'll write it myself if/when I end up applying some version
of it. I've wasted enough time trying to get my point across.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

[Arvind Sankar]

Prior to binutils-2.23, ld treats the location counter as absolute if
used outside an output section definition. From version 2.23 onwards,
the location counter is treated as relative to an adjacent output
section (usually the previous one, unless there isn't one or the
location counter has been assigned to previously, in which case the next
one).

The result is that a symbol definition in the linker script, such as
	_etext = .;
that appears outside an output section definition makes _etext an
absolute symbol prior to binutils-2.23 and a relative symbol from
version 2.23 onwards. So when using a 2.21 or 2.22 vintage linker, the
build fails with
	Invalid absolute R_X86_64_32S relocation: _etext
for x86-64, and a similar message with R_386_32 for x86-32.

This can be reproduced with the official 2.21.1 and 2.22 binutils
releases.

Commit b907693883fd ("x86/vmlinux: Actually use _etext for the end of
the text segment") moved _etext out of the .text section to place it
after the exception table, however since commit f0d7ee17d57c
("x86/vmlinux: Move EXCEPTION_TABLE to RO_DATA segment") this is no
longer needed. Move _etext back inside .text to make it relative even
with older linkers.

Commit c603a309cc75 ("x86/mm: Identify the end of the kernel area to be
reserved") defines __end_of_kernel_reserve using the location counter
outside an output section definition. Use __bss_stop instead of the
location counter for the definition to make it relative with older
linkers.

Fixes: b907693883fd ("x86/vmlinux: Actually use _etext for the end of the text segment")
Fixes: c603a309cc75 ("x86/mm: Identify the end of the kernel area to be reserved")
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
v3: Modify vmlinux.lds.S instead of adding more workarounds to tools/relocs.c

 arch/x86/kernel/vmlinux.lds.S | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 3a1a819da137..bad4e22384dc 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -144,10 +144,12 @@ SECTIONS
 		*(.text.__x86.indirect_thunk)
 		__indirect_thunk_end = .;
 #endif
+
+		/* End of text section */
+		_etext = .;
 	} :text =0xcccc
 
-	/* End of text section, which should occupy whole number of pages */
-	_etext = .;
+	/* .text should occupy whole number of pages */
 	. = ALIGN(PAGE_SIZE);
 
 	X86_ALIGN_RODATA_BEGIN
@@ -372,7 +374,7 @@ SECTIONS
 	 * explicitly reserved using memblock_reserve() or it will be discarded
 	 * and treated as available memory.
 	 */
-	__end_of_kernel_reserve = .;
+	__end_of_kernel_reserve = __bss_stop;
 
 	. = ALIGN(PAGE_SIZE);
 	.brk : AT(ADDR(.brk) - LOAD_OFFSET) {
-- 
2.24.1

Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

[Tom Lendacky]

On 1/13/20 1:53 PM, Arvind Sankar wrote:
> Prior to binutils-2.23, ld treats the location counter as absolute if
> used outside an output section definition. From version 2.23 onwards,
> the location counter is treated as relative to an adjacent output
> section (usually the previous one, unless there isn't one or the
> location counter has been assigned to previously, in which case the next
> one).
> 
> The result is that a symbol definition in the linker script, such as
> 	_etext = .;
> that appears outside an output section definition makes _etext an
> absolute symbol prior to binutils-2.23 and a relative symbol from
> version 2.23 onwards. So when using a 2.21 or 2.22 vintage linker, the
> build fails with
> 	Invalid absolute R_X86_64_32S relocation: _etext
> for x86-64, and a similar message with R_386_32 for x86-32.
> 
> This can be reproduced with the official 2.21.1 and 2.22 binutils
> releases.
> 
> Commit b907693883fd ("x86/vmlinux: Actually use _etext for the end of
> the text segment") moved _etext out of the .text section to place it
> after the exception table, however since commit f0d7ee17d57c
> ("x86/vmlinux: Move EXCEPTION_TABLE to RO_DATA segment") this is no
> longer needed. Move _etext back inside .text to make it relative even
> with older linkers.
> 
> Commit c603a309cc75 ("x86/mm: Identify the end of the kernel area to be
> reserved") defines __end_of_kernel_reserve using the location counter
> outside an output section definition. Use __bss_stop instead of the
> location counter for the definition to make it relative with older
> linkers.
> 
> Fixes: b907693883fd ("x86/vmlinux: Actually use _etext for the end of the text segment")
> Fixes: c603a309cc75 ("x86/mm: Identify the end of the kernel area to be reserved")
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
> v3: Modify vmlinux.lds.S instead of adding more workarounds to tools/relocs.c
> 
>  arch/x86/kernel/vmlinux.lds.S | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index 3a1a819da137..bad4e22384dc 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -144,10 +144,12 @@ SECTIONS
>  		*(.text.__x86.indirect_thunk)
>  		__indirect_thunk_end = .;
>  #endif
> +
> +		/* End of text section */
> +		_etext = .;
>  	} :text =0xcccc
>  
> -	/* End of text section, which should occupy whole number of pages */
> -	_etext = .;
> +	/* .text should occupy whole number of pages */
>  	. = ALIGN(PAGE_SIZE);
>  
>  	X86_ALIGN_RODATA_BEGIN
> @@ -372,7 +374,7 @@ SECTIONS
>  	 * explicitly reserved using memblock_reserve() or it will be discarded
>  	 * and treated as available memory.
>  	 */
> -	__end_of_kernel_reserve = .;
> +	__end_of_kernel_reserve = __bss_stop;

The only thing I worry about is if someone inserts a section between
the bss section and here and doesn't update this value.

Thanks,
Tom

>  
>  	. = ALIGN(PAGE_SIZE);
>  	.brk : AT(ADDR(.brk) - LOAD_OFFSET) {
> 

Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

[Arvind Sankar]

On Mon, Jan 13, 2020 at 03:46:04PM -0600, Tom Lendacky wrote:
> > @@ -372,7 +374,7 @@ SECTIONS
> >  	 * explicitly reserved using memblock_reserve() or it will be discarded
> >  	 * and treated as available memory.
> >  	 */
> > -	__end_of_kernel_reserve = .;
> > +	__end_of_kernel_reserve = __bss_stop;
> 
> The only thing I worry about is if someone inserts a section between
> the bss section and here and doesn't update this value.
> 
> Thanks,
> Tom

Could do something like this, or maybe with just the comment?

diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index bad4e22384dc..25aa1b30068e 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -373,9 +373,16 @@ SECTIONS
 	 * automatically reserved in setup_arch(). Anything after here must be
 	 * explicitly reserved using memblock_reserve() or it will be discarded
 	 * and treated as available memory.
+	 *
+	 * Note that this is defined to use __bss_stop rather than dot for
+	 * compatibility with binutils <2.23, so the definition MUST be updated
+	 * if another section is added after .bss.
 	 */
 	__end_of_kernel_reserve = __bss_stop;
 
+	/* Used for build-time check that __end_of_kernel_reserve is correct */
+	__end_of_kernel_reserve_check = .;
+
 	. = ALIGN(PAGE_SIZE);
 	.brk : AT(ADDR(.brk) - LOAD_OFFSET) {
 		__brk_base = .;
@@ -451,6 +458,9 @@ INIT_PER_CPU(irq_stack_backing_store);
 
 #endif /* CONFIG_X86_32 */
 
+. = ASSERT(__end_of_kernel_reserve == __end_of_kernel_reserve_check,
+	   "__end_of_kernel_reserve is incorrectly defined");
+
 #ifdef CONFIG_KEXEC_CORE
 #include <asm/kexec.h>
 

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

[Arvind Sankar]

On Sat, Jan 11, 2020 at 02:02:43PM +0100, Borislav Petkov wrote:
> On Fri, Jan 10, 2020 at 03:50:29PM -0500, Arvind Sankar wrote:
> > On Fri, Jan 10, 2020 at 09:38:28PM +0100, Borislav Petkov wrote:
> > > On Fri, Jan 10, 2020 at 03:23:49PM -0500, Arvind Sankar wrote:
> > > > Pre-2.23 binutils makes symbols defined outside sections absolute, so
> > > > these two symbols break the build on old linkers.
> > > 
> > > -ENOTENOUGHINFO
> > > 
> > > Which old linkers, how exactly do they break the build, etc etc?
> > > 
> > > Please give exact reproduction steps.
> > > 
> > > Thx.
> > > 
> > 
> > binutils-2.21 and -2.22. An x86-64 defconfig will fail with
> > 	Invalid absolute R_X86_64_32S relocation: _etext
> > and after fixing that one, with
> > 	Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve
> 
> I'm still not clear as to why this happens. I tried reproducing on
> openSUSE 12.1 which has
> 
> Repository: openSUSE-12.1-Oss
> Name: binutils
> Version: 2.21.1-12.1.4
> 
> and the build there fails with:
> 
> objdump: arch/x86/lib/clear_page_64.o: File format not recognized
> objdump: arch/x86/lib/cmdline.o: File format not recognized
> objdump: arch/x86/lib/cmpxchg16b_emu.o: File format not recognized
> objdump: arch/x86/lib/copy_page_64.o: File format not recognized
> objdump: arch/x86/lib/copy_user_64.o: File format not recognized
> objdump: arch/x86/lib/cpu.o: File format not recognized
> ...
> 
> and objdump is part of binutils.
> 
> Now, this looks like another symptom of what you're reporting but what
> we're missing is the rootcause about *why* this happens.

I tried out OpenSUSE 12.1 in a VM, and this one seems to be related to
something that changes in the build when libelf-devel is installed.
Deleting that package and switching UNWINDER_ORC (which says it requires
libelf-devel) to UNWINDER_FRAME_POINTER builds without the file format
errors.

> 
> Because if the issue is hard to fix or similar, then we probably should
> raise the minimum supported binutils version from 2.21 to something
> newer and not do this fix.
> 
> But before we do that, we need a proper analysis as to why it happens.
> 
> Also, what distro are you using to reproduce it on?
> 
> Thx.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

[Kees Cook]

On Mon, Jan 13, 2020 at 02:53:37PM -0500, Arvind Sankar wrote:
> Prior to binutils-2.23, ld treats the location counter as absolute if
> used outside an output section definition. From version 2.23 onwards,
> the location counter is treated as relative to an adjacent output
> section (usually the previous one, unless there isn't one or the
> location counter has been assigned to previously, in which case the next
> one).
> 
> The result is that a symbol definition in the linker script, such as
> 	_etext = .;
> that appears outside an output section definition makes _etext an
> absolute symbol prior to binutils-2.23 and a relative symbol from
> version 2.23 onwards. So when using a 2.21 or 2.22 vintage linker, the
> build fails with
> 	Invalid absolute R_X86_64_32S relocation: _etext
> for x86-64, and a similar message with R_386_32 for x86-32.
> 
> This can be reproduced with the official 2.21.1 and 2.22 binutils
> releases.
> 
> Commit b907693883fd ("x86/vmlinux: Actually use _etext for the end of
> the text segment") moved _etext out of the .text section to place it
> after the exception table, however since commit f0d7ee17d57c
> ("x86/vmlinux: Move EXCEPTION_TABLE to RO_DATA segment") this is no
> longer needed. Move _etext back inside .text to make it relative even
> with older linkers.
> 
> Commit c603a309cc75 ("x86/mm: Identify the end of the kernel area to be
> reserved") defines __end_of_kernel_reserve using the location counter
> outside an output section definition. Use __bss_stop instead of the
> location counter for the definition to make it relative with older
> linkers.
> 
> Fixes: b907693883fd ("x86/vmlinux: Actually use _etext for the end of the text segment")
> Fixes: c603a309cc75 ("x86/mm: Identify the end of the kernel area to be reserved")
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
> v3: Modify vmlinux.lds.S instead of adding more workarounds to tools/relocs.c
> 
>  arch/x86/kernel/vmlinux.lds.S | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index 3a1a819da137..bad4e22384dc 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -144,10 +144,12 @@ SECTIONS
>  		*(.text.__x86.indirect_thunk)
>  		__indirect_thunk_end = .;
>  #endif
> +
> +		/* End of text section */
> +		_etext = .;
>  	} :text =0xcccc
>  
> -	/* End of text section, which should occupy whole number of pages */
> -	_etext = .;
> +	/* .text should occupy whole number of pages */
>  	. = ALIGN(PAGE_SIZE);

NAK: linkers can add things at the end of .text that will go missing from
the kernel if _etext isn't _outside_ the .text section, truly beyond the
end of the .text section. This patch will break Control Flow Integrity
checking since the jump tables are at the end of .text.

Boris, we're always working around weird linker problems; I don't see a
problem with the v2 patch to fix up old binutils...

-Kees

>  
>  	X86_ALIGN_RODATA_BEGIN
> @@ -372,7 +374,7 @@ SECTIONS
>  	 * explicitly reserved using memblock_reserve() or it will be discarded
>  	 * and treated as available memory.
>  	 */
> -	__end_of_kernel_reserve = .;
> +	__end_of_kernel_reserve = __bss_stop;
>  
>  	. = ALIGN(PAGE_SIZE);
>  	.brk : AT(ADDR(.brk) - LOAD_OFFSET) {
> -- 
> 2.24.1
> 

-- 
Kees Cook

Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

[H. Peter Anvin]

On 2020-01-13 17:53, Kees Cook wrote:>>
>> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
>> index 3a1a819da137..bad4e22384dc 100644
>> --- a/arch/x86/kernel/vmlinux.lds.S
>> +++ b/arch/x86/kernel/vmlinux.lds.S
>> @@ -144,10 +144,12 @@ SECTIONS
>>  		*(.text.__x86.indirect_thunk)
>>  		__indirect_thunk_end = .;
>>  #endif
>> +
>> +		/* End of text section */
>> +		_etext = .;
>>  	} :text =0xcccc
>>  
>> -	/* End of text section, which should occupy whole number of pages */
>> -	_etext = .;
>> +	/* .text should occupy whole number of pages */
>>  	. = ALIGN(PAGE_SIZE);
> 
> NAK: linkers can add things at the end of .text that will go missing from
> the kernel if _etext isn't _outside_ the .text section, truly beyond the
> end of the .text section. This patch will break Control Flow Integrity
> checking since the jump tables are at the end of .text.
> 
> Boris, we're always working around weird linker problems; I don't see a
> problem with the v2 patch to fix up old binutils...
> 

Why not add the marker into a separate section instead of leaving it as an
absolute "floater"? Very old binutils would botch that case, but I think that
has been long since addressed well below our current minimum version.

	-hpa

Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

[Kees Cook]

On Mon, Jan 13, 2020 at 05:57:23PM -0800, H. Peter Anvin wrote:
> On 2020-01-13 17:53, Kees Cook wrote:>>
> >> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> >> index 3a1a819da137..bad4e22384dc 100644
> >> --- a/arch/x86/kernel/vmlinux.lds.S
> >> +++ b/arch/x86/kernel/vmlinux.lds.S
> >> @@ -144,10 +144,12 @@ SECTIONS
> >>  		*(.text.__x86.indirect_thunk)
> >>  		__indirect_thunk_end = .;
> >>  #endif
> >> +
> >> +		/* End of text section */
> >> +		_etext = .;
> >>  	} :text =0xcccc
> >>  
> >> -	/* End of text section, which should occupy whole number of pages */
> >> -	_etext = .;
> >> +	/* .text should occupy whole number of pages */
> >>  	. = ALIGN(PAGE_SIZE);
> > 
> > NAK: linkers can add things at the end of .text that will go missing from
> > the kernel if _etext isn't _outside_ the .text section, truly beyond the
> > end of the .text section. This patch will break Control Flow Integrity
> > checking since the jump tables are at the end of .text.
> > 
> > Boris, we're always working around weird linker problems; I don't see a
> > problem with the v2 patch to fix up old binutils...
> > 
> 
> Why not add the marker into a separate section instead of leaving it as an
> absolute "floater"? Very old binutils would botch that case, but I think that
> has been long since addressed well below our current minimum version.

Can you send a patch with what you're thinking?

-- 
Kees Cook

Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

[Arvind Sankar]

On Mon, Jan 13, 2020 at 05:57:23PM -0800, H. Peter Anvin wrote:
> On 2020-01-13 17:53, Kees Cook wrote:>>
> >> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> >> index 3a1a819da137..bad4e22384dc 100644
> >> --- a/arch/x86/kernel/vmlinux.lds.S
> >> +++ b/arch/x86/kernel/vmlinux.lds.S
> >> @@ -144,10 +144,12 @@ SECTIONS
> >>  		*(.text.__x86.indirect_thunk)
> >>  		__indirect_thunk_end = .;
> >>  #endif
> >> +
> >> +		/* End of text section */
> >> +		_etext = .;
> >>  	} :text =0xcccc
> >>  
> >> -	/* End of text section, which should occupy whole number of pages */
> >> -	_etext = .;
> >> +	/* .text should occupy whole number of pages */
> >>  	. = ALIGN(PAGE_SIZE);
> > 
> > NAK: linkers can add things at the end of .text that will go missing from
> > the kernel if _etext isn't _outside_ the .text section, truly beyond the
> > end of the .text section. This patch will break Control Flow Integrity
> > checking since the jump tables are at the end of .text.
> > 
> > Boris, we're always working around weird linker problems; I don't see a
> > problem with the v2 patch to fix up old binutils...
> > 
> 
> Why not add the marker into a separate section instead of leaving it as an
> absolute "floater"? Very old binutils would botch that case, but I think that
> has been long since addressed well below our current minimum version.
> 
> 	-hpa
> 
> 
> 

Kees, thanks, I noted in the other email that you had mentioned this in
a since-reverted commit, but you did not mention in the most recent
commit.

hpa, I think this runs afoul of the bug you noted in commit fd952815307f
("x86-32, relocs: Whitelist more symbols for ld bug workaround"), ld
version 2.22.52.0.[12] can incorrectly promote relative symbols to
absolute, if the output section they appear in is otherwise empty.

That's 2.22, which is more recent than the 2.21 that the kernel
documents as supported.

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

[Arvind Sankar]

On Mon, Jan 13, 2020 at 05:38:55PM +0100, Borislav Petkov wrote:
> On Mon, Jan 13, 2020 at 11:13:10AM -0500, Arvind Sankar wrote:
> > 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:
> 
> And we should try not to do hacks, if it can be fixed properly, as
> binutils expects symbols to be usually relative to a section.
> 

I've poked around a bit more, and all the hacks in relocs.c in S_REL,
other than init_per_cpu__.*, are only there to work around old and/or
buggy binutils versions from the 2.21-2.22 days.

The current code is *not* broken for binutils-2.23+, where none of these
symbols are marked as absolute. IOW, the current code is perfectly fine
and generates relative symbols for any binutils not of 2.21-2.22
vintage.

So we have the following choices:
* bump minimum supported binutils version to 2.23, and remove all the
  hacks for earlier versions.
* add a couple of hacks to relocs.c to account for new symbols, as in v2
* complicate regular code, as in v3, which Kees says might break new
  code [1] and Tom says is bad for future kernel hackers [2]

[1] https://lore.kernel.org/lkml/202001131750.C1B8468@keescook/
[2] https://lore.kernel.org/lkml/3e46154d-6e5e-1c16-90fe-f2c5daa44b60@amd.com/

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

[Arvind Sankar]

On Mon, Jan 13, 2020 at 07:08:26PM +0100, Borislav Petkov wrote:
> On Mon, Jan 13, 2020 at 12:59:38PM -0500, Arvind Sankar wrote:
> > How is "breaks with binutils before version 2.23" not clear enough? It
> 
> Nevermind, I'll write it myself if/when I end up applying some version
> of it. I've wasted enough time trying to get my point across.
> 

You're wasting time, because you're not _listening_ to the other guy.
There is nothing special about how to repro this bug. It can be repro'd
by building binutils-2.21.1 from source, it can be repro'd by building
with a distro that packaged 2.21 -- such as OpenSUSE 12.1 [1], it can be
repro'd by Mauro doing an Android build [2].

The common thread in all that, is that the kernel is being built with a
binutils prior to version 2.23, which is what is noted in the commit log
as it stands.

[1] https://lore.kernel.org/lkml/20200113233829.GB2000869@rani.riverdale.lan/
[2] https://lore.kernel.org/lkml/CAEQFVGbWKf2ksMrMmtymewArSF=ztNgeuieEQ3wvKrX1r759Aw@mail.gmail.com/

Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

[hpa]

On January 13, 2020 7:58:59 PM PST, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>On Mon, Jan 13, 2020 at 05:57:23PM -0800, H. Peter Anvin wrote:
>> On 2020-01-13 17:53, Kees Cook wrote:>>
>> >> diff --git a/arch/x86/kernel/vmlinux.lds.S
>b/arch/x86/kernel/vmlinux.lds.S
>> >> index 3a1a819da137..bad4e22384dc 100644
>> >> --- a/arch/x86/kernel/vmlinux.lds.S
>> >> +++ b/arch/x86/kernel/vmlinux.lds.S
>> >> @@ -144,10 +144,12 @@ SECTIONS
>> >>  		*(.text.__x86.indirect_thunk)
>> >>  		__indirect_thunk_end = .;
>> >>  #endif
>> >> +
>> >> +		/* End of text section */
>> >> +		_etext = .;
>> >>  	} :text =0xcccc
>> >>  
>> >> -	/* End of text section, which should occupy whole number of
>pages */
>> >> -	_etext = .;
>> >> +	/* .text should occupy whole number of pages */
>> >>  	. = ALIGN(PAGE_SIZE);
>> > 
>> > NAK: linkers can add things at the end of .text that will go
>missing from
>> > the kernel if _etext isn't _outside_ the .text section, truly
>beyond the
>> > end of the .text section. This patch will break Control Flow
>Integrity
>> > checking since the jump tables are at the end of .text.
>> > 
>> > Boris, we're always working around weird linker problems; I don't
>see a
>> > problem with the v2 patch to fix up old binutils...
>> > 
>> 
>> Why not add the marker into a separate section instead of leaving it
>as an
>> absolute "floater"? Very old binutils would botch that case, but I
>think that
>> has been long since addressed well below our current minimum version.
>> 
>> 	-hpa
>> 
>> 
>> 
>
>Kees, thanks, I noted in the other email that you had mentioned this in
>a since-reverted commit, but you did not mention in the most recent
>commit.
>
>hpa, I think this runs afoul of the bug you noted in commit
>fd952815307f
>("x86-32, relocs: Whitelist more symbols for ld bug workaround"), ld
>version 2.22.52.0.[12] can incorrectly promote relative symbols to
>absolute, if the output section they appear in is otherwise empty.
>
>That's 2.22, which is more recent than the 2.21 that the kernel
>documents as supported.

Oh right... they introduced that little piece of drain bramage.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

[Borislav Petkov]

On Mon, Jan 13, 2020 at 11:17:25PM -0500, Arvind Sankar wrote:
> On Mon, Jan 13, 2020 at 07:08:26PM +0100, Borislav Petkov wrote:
> > On Mon, Jan 13, 2020 at 12:59:38PM -0500, Arvind Sankar wrote:
> > > How is "breaks with binutils before version 2.23" not clear enough? It
> > 
> > Nevermind, I'll write it myself if/when I end up applying some version
> > of it. I've wasted enough time trying to get my point across.
> > 
> 
> You're wasting time, because you're not _listening_ to the other guy.

FFS you're still missing the point: the question is whether this
is a widespread issue - a distro shipping this funky binutils and
therefore it being a problem on potentially more than one environment -
or something people can only trigger by *specially* building themselves
and thus a lot more seldom occurrence.

And I've answered the question myself by booting openSUSE 12.1 - i.e.,
at least one distro has it.

And regardless, so what if you add some more text to the commit message?
Are you afraid that you'll over-describe the issue? Hell, you've typed
more just in debating this.

And let me tell you why it is a good thing to have more detailed
explanations in commit messages: when you move on and go do something
else, all that is left is a commit message for maintainers to do git
archaeology on and scratch heads as to why stuff was done the way it was.

And this happens very often: read a oneliner commit message and go find
a crystal ball to figure out what the author meant.

And I told you that *I* will write it myself and you wouldn't have to do
diddly squat. And yet you still can't let it go!

So you can debate all you want - you won't change my mind about wanting
to have stuff explained in detail in commit messages.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Arvind Sankar]

On Tue, Jan 14, 2020 at 12:25:38PM +0100, Borislav Petkov wrote:
> 
> FFS you're still missing the point: the question is whether this
> is a widespread issue - a distro shipping this funky binutils and
> therefore it being a problem on potentially more than one environment -
> or something people can only trigger by *specially* building themselves
> and thus a lot more seldom occurrence.
> 
> And I've answered the question myself by booting openSUSE 12.1 - i.e.,
> at least one distro has it.

This is just a misunderstanding on what you were asking to be added
then. I have no objections to noting that popular distros have shipped
with those problematic binutils versions, so keeping them in mind is
important.

Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

[Borislav Petkov]

On Mon, Jan 13, 2020 at 05:53:32PM -0800, Kees Cook wrote:
> NAK: linkers can add things at the end of .text that will go missing from
> the kernel if _etext isn't _outside_ the .text section, truly beyond the
> end of the .text section. This patch will break Control Flow Integrity
> checking since the jump tables are at the end of .text.

Err, which linkers are those? Please elaborate.

In any case, after reading the thread, I can't help but favor the idea
of us bumping min binutils version to 2.23.

Michael (on Cc) says that the 2.21 was kinda broken wrt to the symbols
fun outside of sections, 2.22 tried to fix it, see

  fd952815307f ("x86-32, relocs: Whitelist more symbols for ld bug workaround")

which Arvind pointed out and 2.23 fixed it for real.

Now, 2.23 is still very ancient. I'm looking at our releases: openSUSE
12.1 has the minimum supported gcc version 4.6 by the kernel and
also the minimum binutils version 2.21 which we support according to
Documentation/process/changes.rst

Now, openSUSE 12.1 is ancient and we ourselves advise people to update
to current distros so I don't think anyone would still run it.

So, considering that upping the binutils version would save us from all
this trouble I say we try it after 5.5 releases for a maximum time of a
full 5.6 release cycle and see who complains.

Considering how no one triggered this yet until Arvind, I think no one
would complain. But I might be wrong.

So what do people think? hpa?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

[hpa]

On January 14, 2020 8:51:35 AM PST, Borislav Petkov <bp@alien8.de> wrote:
>On Mon, Jan 13, 2020 at 05:53:32PM -0800, Kees Cook wrote:
>> NAK: linkers can add things at the end of .text that will go missing
>from
>> the kernel if _etext isn't _outside_ the .text section, truly beyond
>the
>> end of the .text section. This patch will break Control Flow
>Integrity
>> checking since the jump tables are at the end of .text.
>
>Err, which linkers are those? Please elaborate.
>
>In any case, after reading the thread, I can't help but favor the idea
>of us bumping min binutils version to 2.23.
>
>Michael (on Cc) says that the 2.21 was kinda broken wrt to the symbols
>fun outside of sections, 2.22 tried to fix it, see
>
>fd952815307f ("x86-32, relocs: Whitelist more symbols for ld bug
>workaround")
>
>which Arvind pointed out and 2.23 fixed it for real.
>
>Now, 2.23 is still very ancient. I'm looking at our releases: openSUSE
>12.1 has the minimum supported gcc version 4.6 by the kernel and
>also the minimum binutils version 2.21 which we support according to
>Documentation/process/changes.rst
>
>Now, openSUSE 12.1 is ancient and we ourselves advise people to update
>to current distros so I don't think anyone would still run it.
>
>So, considering that upping the binutils version would save us from all
>this trouble I say we try it after 5.5 releases for a maximum time of a
>full 5.6 release cycle and see who complains.
>
>Considering how no one triggered this yet until Arvind, I think no one
>would complain. But I might be wrong.
>
>So what do people think? hpa?

I'm all for dumping support for an ancient, known buggy binutils.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

[Arvind Sankar]

On Tue, Jan 14, 2020 at 05:51:35PM +0100, Borislav Petkov wrote:
> On Mon, Jan 13, 2020 at 05:53:32PM -0800, Kees Cook wrote:
> > NAK: linkers can add things at the end of .text that will go missing from
> > the kernel if _etext isn't _outside_ the .text section, truly beyond the
> > end of the .text section. This patch will break Control Flow Integrity
> > checking since the jump tables are at the end of .text.
> 
> Err, which linkers are those? Please elaborate.
> 
> In any case, after reading the thread, I can't help but favor the idea
> of us bumping min binutils version to 2.23.
> 
> Michael (on Cc) says that the 2.21 was kinda broken wrt to the symbols
> fun outside of sections, 2.22 tried to fix it, see
> 
>   fd952815307f ("x86-32, relocs: Whitelist more symbols for ld bug workaround")
> 
> which Arvind pointed out and 2.23 fixed it for real.
> 
> Now, 2.23 is still very ancient. I'm looking at our releases: openSUSE
> 12.1 has the minimum supported gcc version 4.6 by the kernel and
> also the minimum binutils version 2.21 which we support according to
> Documentation/process/changes.rst
> 
> Now, openSUSE 12.1 is ancient and we ourselves advise people to update
> to current distros so I don't think anyone would still run it.

RHEL7 looks to have been released with 2.23 and we already don't
support the version in RHEL6, so that should be good too.

> 
> So, considering that upping the binutils version would save us from all
> this trouble I say we try it after 5.5 releases for a maximum time of a
> full 5.6 release cycle and see who complains.
> 
> Considering how no one triggered this yet until Arvind, I think no one
> would complain. But I might be wrong.
> 
> So what do people think? hpa?
> 

Thumbs up from me -- I had thought there were a few other reports
earlier about these, but looking at those threads, it seems like they're
all actually with ld.gold, which we already decided to drop support for
in commit 75959d44f9dc ("kbuild: Fail if gold linker is detected").

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

Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

[Borislav Petkov]

On Tue, Jan 14, 2020 at 07:21:32PM -0500, Arvind Sankar wrote:
> RHEL7 looks to have been released with 2.23 and we already don't
> support the version in RHEL6, so that should be good too.

Good.

> Thumbs up from me -- I had thought there were a few other reports
> earlier about these,

Lemme clarify what I meant: I'm not aware of any other reports wrt
current kernel and ancient binutils build issues.

> but looking at those threads, it seems like they're
> all actually with ld.gold, which we already decided to drop support for
> in commit 75959d44f9dc ("kbuild: Fail if gold linker is detected").

Ok, then. So I'd say, we should do a trivial patch upping the binutils
version in Documentation/process/changes.rst with an explanation why
we're doing it. I'll queue it after 5.6-rc1 into tip and then it would
spend the maximal amount of exposure time in linux-next until it
releases in 5.7 sometime in the late summer.

I think that's the maximal exposure we can do before it hits an
upstream, non-rc release and hope that whoever wants to complain, will
do so before then.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Borislav Petkov]

Long overdue patch, see below.

Plan is to queue it after 5.7-rc1.

---
From: Borislav Petkov <bp@suse.de>
Date: Mon, 16 Mar 2020 16:28:36 +0100
Subject: [PATCH] Documentation/changes: Raise minimum supported binutilsa version to 2.23

The currently minimum-supported binutils version 2.21 has the problem of
promoting symbols which are defined outside of a section into absolute.
According to Arvind:

  binutils-2.21 and -2.22. An x86-64 defconfig will fail with
          Invalid absolute R_X86_64_32S relocation: _etext
  and after fixing that one, with
          Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve

Those two versions of binutils have a bug when it comes to handling
symbols defined outside of a section and binutils 2.23 has the proper
fix, see: https://sourceware.org/legacy-ml/binutils/2012-06/msg00155.html

Therefore, up to the fixed version directly, skipping the broken ones.

Currently shipping distros already have the fixed binutils version so
there should be no breakage resulting from this.

For more details about the whole thing, see the thread in Link.

Signed-off-by: Borislav Petkov <bp@suse.de>
Link: https://lkml.kernel.org/r/20200110202349.1881840-1-nivedita@alum.mit.edu
---
 Documentation/process/changes.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
index e47863575917..7a842655142c 100644
--- a/Documentation/process/changes.rst
+++ b/Documentation/process/changes.rst
@@ -31,7 +31,7 @@ you probably needn't concern yourself with pcmciautils.
 ====================== ===============  ========================================
 GNU C                  4.6              gcc --version
 GNU make               3.81             make --version
-binutils               2.21             ld -v
+binutils               2.23             ld -v
 flex                   2.5.35           flex --version
 bison                  2.0              bison --version
 util-linux             2.10o            fdformat --version
-- 
2.21.0

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Kees Cook]

On Mon, Mar 16, 2020 at 05:02:59PM +0100, Borislav Petkov wrote:
> Long overdue patch, see below.
> 
> Plan is to queue it after 5.7-rc1.
> 
> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Mon, 16 Mar 2020 16:28:36 +0100
> Subject: [PATCH] Documentation/changes: Raise minimum supported binutilsa version to 2.23
> 
> The currently minimum-supported binutils version 2.21 has the problem of
> promoting symbols which are defined outside of a section into absolute.
> According to Arvind:
> 
>   binutils-2.21 and -2.22. An x86-64 defconfig will fail with
>           Invalid absolute R_X86_64_32S relocation: _etext
>   and after fixing that one, with
>           Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve
> 
> Those two versions of binutils have a bug when it comes to handling
> symbols defined outside of a section and binutils 2.23 has the proper
> fix, see: https://sourceware.org/legacy-ml/binutils/2012-06/msg00155.html
> 
> Therefore, up to the fixed version directly, skipping the broken ones.
> 
> Currently shipping distros already have the fixed binutils version so
> there should be no breakage resulting from this.
> 
> For more details about the whole thing, see the thread in Link.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> Link: https://lkml.kernel.org/r/20200110202349.1881840-1-nivedita@alum.mit.edu
> ---
>  Documentation/process/changes.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
> index e47863575917..7a842655142c 100644
> --- a/Documentation/process/changes.rst
> +++ b/Documentation/process/changes.rst
> @@ -31,7 +31,7 @@ you probably needn't concern yourself with pcmciautils.
>  ====================== ===============  ========================================
>  GNU C                  4.6              gcc --version
>  GNU make               3.81             make --version
> -binutils               2.21             ld -v
> +binutils               2.23             ld -v
>  flex                   2.5.35           flex --version
>  bison                  2.0              bison --version
>  util-linux             2.10o            fdformat --version
> -- 
> 2.21.0
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

-- 
Kees Cook

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Jason A. Donenfeld]

On Mon, Mar 16, 2020 at 05:02:59PM +0100, Borislav Petkov wrote:
> Long overdue patch, see below.
> 
> Plan is to queue it after 5.7-rc1.
> 
> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Mon, 16 Mar 2020 16:28:36 +0100
> Subject: [PATCH] Documentation/changes: Raise minimum supported binutilsa version to 2.23
> 
> The currently minimum-supported binutils version 2.21 has the problem of
> promoting symbols which are defined outside of a section into absolute.
> According to Arvind:
> 
>   binutils-2.21 and -2.22. An x86-64 defconfig will fail with
>           Invalid absolute R_X86_64_32S relocation: _etext
>   and after fixing that one, with
>           Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve
> 
> Those two versions of binutils have a bug when it comes to handling
> symbols defined outside of a section and binutils 2.23 has the proper
> fix, see: https://sourceware.org/legacy-ml/binutils/2012-06/msg00155.html
> 
> Therefore, up to the fixed version directly, skipping the broken ones.
> 
> Currently shipping distros already have the fixed binutils version so
> there should be no breakage resulting from this.
> 
> For more details about the whole thing, see the thread in Link.

That sounds very good to me. Then we'll be able to use ADX instructions
without ifdefs.

Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>

Re: [PATCH] Documentation/changes: Raise minimum supported binutilsa version to 2.23

[Nick Desaulniers]

> The currently minimum-supported binutils version 2.21 has the problem of
> promoting symbols which are defined outside of a section into absolute.
> According to Arvind:
> 
>   binutils-2.21 and -2.22. An x86-64 defconfig will fail with
>           Invalid absolute R_X86_64_32S relocation: _etext
>   and after fixing that one, with
>           Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve
> 
> Those two versions of binutils have a bug when it comes to handling
> symbols defined outside of a section and binutils 2.23 has the proper
> fix, see: https://sourceware.org/legacy-ml/binutils/2012-06/msg00155.html
> 
> Therefore, up to the fixed version directly, skipping the broken ones.
> 
> Currently shipping distros already have the fixed binutils version so
> there should be no breakage resulting from this.
> 
> For more details about the whole thing, see the thread in Link.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Link: https://lkml.kernel.org/r/20200110202349.1881840-1-nivedita@alum.mit.edu

Acked-by: Nick Desaulniers <ndesaulniers@google.com>

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Kees Cook]

On Mon, Mar 23, 2020 at 02:44:54PM -0600, Jason A. Donenfeld wrote:
> On Mon, Mar 16, 2020 at 05:02:59PM +0100, Borislav Petkov wrote:
> > Long overdue patch, see below.
> > 
> > Plan is to queue it after 5.7-rc1.
> > 
> > ---
> > From: Borislav Petkov <bp@suse.de>
> > Date: Mon, 16 Mar 2020 16:28:36 +0100
> > Subject: [PATCH] Documentation/changes: Raise minimum supported binutilsa version to 2.23
> > 
> > The currently minimum-supported binutils version 2.21 has the problem of
> > promoting symbols which are defined outside of a section into absolute.
> > According to Arvind:
> > 
> >   binutils-2.21 and -2.22. An x86-64 defconfig will fail with
> >           Invalid absolute R_X86_64_32S relocation: _etext
> >   and after fixing that one, with
> >           Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve
> > 
> > Those two versions of binutils have a bug when it comes to handling
> > symbols defined outside of a section and binutils 2.23 has the proper
> > fix, see: https://sourceware.org/legacy-ml/binutils/2012-06/msg00155.html
> > 
> > Therefore, up to the fixed version directly, skipping the broken ones.
> > 
> > Currently shipping distros already have the fixed binutils version so
> > there should be no breakage resulting from this.
> > 
> > For more details about the whole thing, see the thread in Link.
> 
> That sounds very good to me. Then we'll be able to use ADX instructions
> without ifdefs.
> 
> Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>

Can you send these now and we can land in 5.7 with the doc change?

-Kees

-- 
Kees Cook

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Jason A. Donenfeld]

On Mon, Mar 23, 2020 at 2:51 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Mar 23, 2020 at 02:44:54PM -0600, Jason A. Donenfeld wrote:
> > On Mon, Mar 16, 2020 at 05:02:59PM +0100, Borislav Petkov wrote:
> > > Long overdue patch, see below.
> > >
> > > Plan is to queue it after 5.7-rc1.
> > >
> > > ---
> > > From: Borislav Petkov <bp@suse.de>
> > > Date: Mon, 16 Mar 2020 16:28:36 +0100
> > > Subject: [PATCH] Documentation/changes: Raise minimum supported binutilsa version to 2.23
> > >
> > > The currently minimum-supported binutils version 2.21 has the problem of
> > > promoting symbols which are defined outside of a section into absolute.
> > > According to Arvind:
> > >
> > >   binutils-2.21 and -2.22. An x86-64 defconfig will fail with
> > >           Invalid absolute R_X86_64_32S relocation: _etext
> > >   and after fixing that one, with
> > >           Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve
> > >
> > > Those two versions of binutils have a bug when it comes to handling
> > > symbols defined outside of a section and binutils 2.23 has the proper
> > > fix, see: https://sourceware.org/legacy-ml/binutils/2012-06/msg00155.html
> > >
> > > Therefore, up to the fixed version directly, skipping the broken ones.
> > >
> > > Currently shipping distros already have the fixed binutils version so
> > > there should be no breakage resulting from this.
> > >
> > > For more details about the whole thing, see the thread in Link.
> >
> > That sounds very good to me. Then we'll be able to use ADX instructions
> > without ifdefs.
> >
> > Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>
>
> Can you send these now and we can land in 5.7 with the doc change?

By the way, while we're in the process of updating dependencies, what
if we ratched the minimum binutils on x86 up to 2.25 (which is still
quite old)? In this case, we could get rid of *all* of the CONFIG_AS_*
ifdefs throughout.

Jason

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Masahiro Yamada]

Hi.



On Tue, Mar 24, 2020 at 5:51 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Mar 23, 2020 at 02:44:54PM -0600, Jason A. Donenfeld wrote:
> > On Mon, Mar 16, 2020 at 05:02:59PM +0100, Borislav Petkov wrote:
> > > Long overdue patch, see below.
> > >
> > > Plan is to queue it after 5.7-rc1.
> > >
> > > ---
> > > From: Borislav Petkov <bp@suse.de>
> > > Date: Mon, 16 Mar 2020 16:28:36 +0100
> > > Subject: [PATCH] Documentation/changes: Raise minimum supported binutilsa version to 2.23
> > >
> > > The currently minimum-supported binutils version 2.21 has the problem of
> > > promoting symbols which are defined outside of a section into absolute.
> > > According to Arvind:
> > >
> > >   binutils-2.21 and -2.22. An x86-64 defconfig will fail with
> > >           Invalid absolute R_X86_64_32S relocation: _etext
> > >   and after fixing that one, with
> > >           Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve
> > >
> > > Those two versions of binutils have a bug when it comes to handling
> > > symbols defined outside of a section and binutils 2.23 has the proper
> > > fix, see: https://sourceware.org/legacy-ml/binutils/2012-06/msg00155.html
> > >
> > > Therefore, up to the fixed version directly, skipping the broken ones.
> > >
> > > Currently shipping distros already have the fixed binutils version so
> > > there should be no breakage resulting from this.
> > >
> > > For more details about the whole thing, see the thread in Link.
> >
> > That sounds very good to me. Then we'll be able to use ADX instructions
> > without ifdefs.
> >
> > Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>
>
> Can you send these now and we can land in 5.7 with the doc change?
>
> -Kees


Kees,

I folded this patch into the following series (16 patches):
https://lore.kernel.org/patchwork/project/lkml/list/?series=435391

This patch (14/16) is a prerequisite for 15/16.



Borislav,

When I forwarded this patch, I fixed up one more line.
(changes.rst duplicates the same information...)

Please see this. I hope this should be OK.
https://lore.kernel.org/patchwork/patch/1214519/



> --
> Kees Cook



--
Best Regards
Masahiro Yamada

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Masahiro Yamada]

On Tue, Mar 24, 2020 at 6:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Hi.
>
>
>
> On Tue, Mar 24, 2020 at 5:51 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Mon, Mar 23, 2020 at 02:44:54PM -0600, Jason A. Donenfeld wrote:
> > > On Mon, Mar 16, 2020 at 05:02:59PM +0100, Borislav Petkov wrote:
> > > > Long overdue patch, see below.
> > > >
> > > > Plan is to queue it after 5.7-rc1.
> > > >
> > > > ---
> > > > From: Borislav Petkov <bp@suse.de>
> > > > Date: Mon, 16 Mar 2020 16:28:36 +0100
> > > > Subject: [PATCH] Documentation/changes: Raise minimum supported binutilsa version to 2.23
> > > >
> > > > The currently minimum-supported binutils version 2.21 has the problem of
> > > > promoting symbols which are defined outside of a section into absolute.
> > > > According to Arvind:
> > > >
> > > >   binutils-2.21 and -2.22. An x86-64 defconfig will fail with
> > > >           Invalid absolute R_X86_64_32S relocation: _etext
> > > >   and after fixing that one, with
> > > >           Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve
> > > >
> > > > Those two versions of binutils have a bug when it comes to handling
> > > > symbols defined outside of a section and binutils 2.23 has the proper
> > > > fix, see: https://sourceware.org/legacy-ml/binutils/2012-06/msg00155.html


This seems to be also related to
7883a14339299773b2ce08dcfd97c63c199a9289


I had noticed the symbol "_text"
was absolute on binutils <= 2.22,
but I was not sure whether it was a bug of the tool.

I applied the fix.
Perhaps, it was unneeded given that
we require the binutils 2.23





> > > >
> > > > Therefore, up to the fixed version directly, skipping the broken ones.
> > > >
> > > > Currently shipping distros already have the fixed binutils version so
> > > > there should be no breakage resulting from this.
> > > >
> > > > For more details about the whole thing, see the thread in Link.
> > >
> > > That sounds very good to me. Then we'll be able to use ADX instructions
> > > without ifdefs.
> > >
> > > Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>
> >
> > Can you send these now and we can land in 5.7 with the doc change?
> >
> > -Kees
>
>
> Kees,
>
> I folded this patch into the following series (16 patches):
> https://lore.kernel.org/patchwork/project/lkml/list/?series=435391
>
> This patch (14/16) is a prerequisite for 15/16.
>
>
>
> Borislav,
>
> When I forwarded this patch, I fixed up one more line.
> (changes.rst duplicates the same information...)
>
> Please see this. I hope this should be OK.
> https://lore.kernel.org/patchwork/patch/1214519/
>
>
>
> > --
> > Kees Cook
>
>
>
> --
> Best Regards
> Masahiro Yamada



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Borislav Petkov]

On Tue, Mar 24, 2020 at 06:02:02PM +0900, Masahiro Yamada wrote:
> Borislav,
> 
> When I forwarded this patch, I fixed up one more line.
> (changes.rst duplicates the same information...)
> 
> Please see this. I hope this should be OK.
> https://lore.kernel.org/patchwork/patch/1214519/

Thanks.

However, I wanted to queue this patch *after* 5.7-rc1 and so that it
lands in 5.8 and so that it has a maximum cycle in testing - well, it is
not really testing but getting more people to see it and have the chance
to complain - and not queue it now.

I did some searching on distrowatch.com last night and the couple of
distros shipping binutils 2.23 I saw, were already EOL but the search
was not exhaustive.

And from looking at your patchset, I think it should get the max time
testing in linux-next too, so that we have time to address any build
issues it might uncover.

IMO.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Masahiro Yamada]

On Tue, Mar 24, 2020 at 6:14 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Tue, Mar 24, 2020 at 06:02:02PM +0900, Masahiro Yamada wrote:
> > Borislav,
> >
> > When I forwarded this patch, I fixed up one more line.
> > (changes.rst duplicates the same information...)
> >
> > Please see this. I hope this should be OK.
> > https://lore.kernel.org/patchwork/patch/1214519/
>
> Thanks.
>
> However, I wanted to queue this patch *after* 5.7-rc1 and so that it
> lands in 5.8 and so that it has a maximum cycle in testing - well, it is
> not really testing but getting more people to see it and have the chance
> to complain - and not queue it now.
>
> I did some searching on distrowatch.com last night and the couple of
> distros shipping binutils 2.23 I saw, were already EOL but the search
> was not exhaustive.

OK, if we take time for this decision,
we can drop 14/16 and 15/16 for now.


> And from looking at your patchset, I think it should get the max time
> testing in linux-next too, so that we have time to address any build
> issues it might uncover.


Linus was positive to have this for 5.7
https://lkml.org/lkml/2020/3/21/262

I hope 01-13 will get merged for the next MW.
We still have a couple of weeks to test it in -next.




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



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Borislav Petkov]

On Tue, Mar 24, 2020 at 06:40:44PM +0900, Masahiro Yamada wrote:
> Linus was positive to have this for 5.7
> https://lkml.org/lkml/2020/3/21/262

Linus acked a 3-patch series there - not that, AFAICT.

> I hope 01-13 will get merged for the next MW.
> We still have a couple of weeks to test it in -next.

We'll see how testing turns out...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Arvind Sankar]

On Tue, Mar 24, 2020 at 06:12:01PM +0900, Masahiro Yamada wrote:
> On Tue, Mar 24, 2020 at 6:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Hi.
> >
> >
> >
> > On Tue, Mar 24, 2020 at 5:51 AM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > On Mon, Mar 23, 2020 at 02:44:54PM -0600, Jason A. Donenfeld wrote:
> > > > On Mon, Mar 16, 2020 at 05:02:59PM +0100, Borislav Petkov wrote:
> > > > > Long overdue patch, see below.
> > > > >
> > > > > Plan is to queue it after 5.7-rc1.
> > > > >
> > > > > ---
> > > > > From: Borislav Petkov <bp@suse.de>
> > > > > Date: Mon, 16 Mar 2020 16:28:36 +0100
> > > > > Subject: [PATCH] Documentation/changes: Raise minimum supported binutilsa version to 2.23
> > > > >
> > > > > The currently minimum-supported binutils version 2.21 has the problem of
> > > > > promoting symbols which are defined outside of a section into absolute.
> > > > > According to Arvind:
> > > > >
> > > > >   binutils-2.21 and -2.22. An x86-64 defconfig will fail with
> > > > >           Invalid absolute R_X86_64_32S relocation: _etext
> > > > >   and after fixing that one, with
> > > > >           Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve
> > > > >
> > > > > Those two versions of binutils have a bug when it comes to handling
> > > > > symbols defined outside of a section and binutils 2.23 has the proper
> > > > > fix, see: https://sourceware.org/legacy-ml/binutils/2012-06/msg00155.html
> 
> 
> This seems to be also related to
> 7883a14339299773b2ce08dcfd97c63c199a9289
> 
> 
> I had noticed the symbol "_text"
> was absolute on binutils <= 2.22,
> but I was not sure whether it was a bug of the tool.
> 
> I applied the fix.
> Perhaps, it was unneeded given that
> we require the binutils 2.23
> 

Which architecture? x86 at least doesn't even build with <= 2.22, but
adding workarounds for that shows _text as section-relative (T in nm
output).

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Jason A. Donenfeld]

On 3/24/20, Borislav Petkov <bp@alien8.de> wrote:
> On Tue, Mar 24, 2020 at 06:02:02PM +0900, Masahiro Yamada wrote:
>> Borislav,
>>
>> When I forwarded this patch, I fixed up one more line.
>> (changes.rst duplicates the same information...)
>>
>> Please see this. I hope this should be OK.
>> https://lore.kernel.org/patchwork/patch/1214519/
>
> Thanks.
>
> However, I wanted to queue this patch *after* 5.7-rc1 and so that it
> lands in 5.8

Both Kees and I were pretty positive yesterday about getting this done
in 5.7, now. I don't think that filing these away in some subsystem
tree will actually result in any more bugs being shaken out, than if
we just do this at the very very beginning of the 5.7 cycle.

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Borislav Petkov]

On Tue, Mar 24, 2020 at 10:22:53AM -0600, Jason A. Donenfeld wrote:
> Both Kees and I were pretty positive yesterday about getting this done
> in 5.7, now.

Are you or Kees going to deal with any fallout from upping the binutils
version, rushed in in the last week before the merge window?

Because I sure as hell am not. *Especially* if there's no big difference
when it goes in.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Linus Torvalds]

On Tue, Mar 24, 2020 at 9:28 AM Borislav Petkov <bp@alien8.de> wrote:
>
> Are you or Kees going to deal with any fallout from upping the binutils
> version, rushed in in the last week before the merge window?

I think it's ok. It's not going to cause any _subtle_ failures, it's
going to cause very clear "oh, now it doesn't build" errors.

No?

And binutils 2.23 is what, 7+ years old by now and apparently had
known failure cases too.

But if there are silent and subtle failures, that might be a reason to
be careful. Are there?

              Linus

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Borislav Petkov]

On Tue, Mar 24, 2020 at 09:37:13AM -0700, Linus Torvalds wrote:
> I think it's ok. It's not going to cause any _subtle_ failures, it's
> going to cause very clear "oh, now it doesn't build" errors.
> 
> No?
> 
> And binutils 2.23 is what, 7+ years old by now and apparently had
> known failure cases too.
> 
> But if there are silent and subtle failures, that might be a reason to
> be careful. Are there?

Well, not that I know of and that's why I'm being overly cautious. Maybe
too cautious but a lot of hectic testing of last minute fixes in the
past have taught me to take my time.

And since it doesn't really matter when the patch goes in - there's
always the next merge window - I would prefer to take our time and have
it simmer in -next for max period.

So yeah, 2.23 has been tested for a long time now and it is very likely
that nothing would happen and if you think it's ok, then sure. Then if
you happen to see urgent pull requests with build or some other fixes,
at least you'll be prepared. :-)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Masahiro Yamada]

On Wed, Mar 25, 2020 at 12:38 AM Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Tue, Mar 24, 2020 at 06:12:01PM +0900, Masahiro Yamada wrote:
> > On Tue, Mar 24, 2020 at 6:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > Hi.
> > >
> > >
> > >
> > > On Tue, Mar 24, 2020 at 5:51 AM Kees Cook <keescook@chromium.org> wrote:
> > > >
> > > > On Mon, Mar 23, 2020 at 02:44:54PM -0600, Jason A. Donenfeld wrote:
> > > > > On Mon, Mar 16, 2020 at 05:02:59PM +0100, Borislav Petkov wrote:
> > > > > > Long overdue patch, see below.
> > > > > >
> > > > > > Plan is to queue it after 5.7-rc1.
> > > > > >
> > > > > > ---
> > > > > > From: Borislav Petkov <bp@suse.de>
> > > > > > Date: Mon, 16 Mar 2020 16:28:36 +0100
> > > > > > Subject: [PATCH] Documentation/changes: Raise minimum supported binutilsa version to 2.23
> > > > > >
> > > > > > The currently minimum-supported binutils version 2.21 has the problem of
> > > > > > promoting symbols which are defined outside of a section into absolute.
> > > > > > According to Arvind:
> > > > > >
> > > > > >   binutils-2.21 and -2.22. An x86-64 defconfig will fail with
> > > > > >           Invalid absolute R_X86_64_32S relocation: _etext
> > > > > >   and after fixing that one, with
> > > > > >           Invalid absolute R_X86_64_32S relocation: __end_of_kernel_reserve
> > > > > >
> > > > > > Those two versions of binutils have a bug when it comes to handling
> > > > > > symbols defined outside of a section and binutils 2.23 has the proper
> > > > > > fix, see: https://sourceware.org/legacy-ml/binutils/2012-06/msg00155.html
> >
> >
> > This seems to be also related to
> > 7883a14339299773b2ce08dcfd97c63c199a9289
> >
> >
> > I had noticed the symbol "_text"
> > was absolute on binutils <= 2.22,
> > but I was not sure whether it was a bug of the tool.
> >
> > I applied the fix.
> > Perhaps, it was unneeded given that
> > we require the binutils 2.23
> >
>
> Which architecture? x86 at least doesn't even build with <= 2.22, but
> adding workarounds for that shows _text as section-relative (T in nm
> output).


The reporter found this for PowerPC,
but I think this could happen on any architecture.

https://patchwork.kernel.org/patch/11430243/




-- 
Best Regards
Masahiro Yamada

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Kees Cook]

On Tue, Mar 24, 2020 at 09:37:13AM -0700, Linus Torvalds wrote:
> On Tue, Mar 24, 2020 at 9:28 AM Borislav Petkov <bp@alien8.de> wrote:
> >
> > Are you or Kees going to deal with any fallout from upping the binutils
> > version, rushed in in the last week before the merge window?
> 
> I think it's ok. It's not going to cause any _subtle_ failures, it's
> going to cause very clear "oh, now it doesn't build" errors.
> 
> No?
> 
> And binutils 2.23 is what, 7+ years old by now and apparently had
> known failure cases too.
> 
> But if there are silent and subtle failures, that might be a reason to
> be careful. Are there?

FWIW, I have plenty of other hills to die on, so I have no urgency on
this change. I actually thought it had already happened, since it was
brought up a while ago. :) I am just excited to see it happen since it
unblocks other work I've been touching. As long as it's "eventually",
I don't care when. :)

-- 
Kees Cook

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Arvind Sankar]

On Wed, Mar 25, 2020 at 02:31:17AM +0900, Masahiro Yamada wrote:
> On Wed, Mar 25, 2020 at 12:38 AM Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > Which architecture? x86 at least doesn't even build with <= 2.22, but
> > adding workarounds for that shows _text as section-relative (T in nm
> > output).
> 
> 
> The reporter found this for PowerPC,
> but I think this could happen on any architecture.
> 
> https://patchwork.kernel.org/patch/11430243/
> 
> 

Ok, the powerpc linker script defines _text outside any output section,
so it is indeed the same issue as with _etext on x86.

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Arvind Sankar]

On Tue, Mar 24, 2020 at 05:48:12PM +0100, Borislav Petkov wrote:
> On Tue, Mar 24, 2020 at 09:37:13AM -0700, Linus Torvalds wrote:
> > I think it's ok. It's not going to cause any _subtle_ failures, it's
> > going to cause very clear "oh, now it doesn't build" errors.
> > 
> > No?
> > 
> > And binutils 2.23 is what, 7+ years old by now and apparently had
> > known failure cases too.
> > 
> > But if there are silent and subtle failures, that might be a reason to
> > be careful. Are there?
> 
> Well, not that I know of and that's why I'm being overly cautious. Maybe
> too cautious but a lot of hectic testing of last minute fixes in the
> past have taught me to take my time.
> 
> And since it doesn't really matter when the patch goes in - there's
> always the next merge window - I would prefer to take our time and have
> it simmer in -next for max period.
> 
> So yeah, 2.23 has been tested for a long time now and it is very likely
> that nothing would happen and if you think it's ok, then sure. Then if
> you happen to see urgent pull requests with build or some other fixes,
> at least you'll be prepared. :-)
> 

This is just a documentation patch right? Nothing actually changes with
the build. The only possible thing that we would potentially have to
deal with is

(1) people noticing the doc change and complaining that they
still need to use binutils-2.21 for some reason -- but they can't
currently build an x86 kernel without patches anyway, so...

(2) people noticing the doc change and suggesting moving to 2.26 or some
later version instead of 2.23.

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Arvind Sankar]

On Tue, Mar 24, 2020 at 05:42:05PM -0400, Arvind Sankar wrote:
> On Tue, Mar 24, 2020 at 05:48:12PM +0100, Borislav Petkov wrote:
> > On Tue, Mar 24, 2020 at 09:37:13AM -0700, Linus Torvalds wrote:
> > > I think it's ok. It's not going to cause any _subtle_ failures, it's
> > > going to cause very clear "oh, now it doesn't build" errors.
> > > 
> > > No?
> > > 
> > > And binutils 2.23 is what, 7+ years old by now and apparently had
> > > known failure cases too.
> > > 
> > > But if there are silent and subtle failures, that might be a reason to
> > > be careful. Are there?
> > 
> > Well, not that I know of and that's why I'm being overly cautious. Maybe
> > too cautious but a lot of hectic testing of last minute fixes in the
> > past have taught me to take my time.
> > 
> > And since it doesn't really matter when the patch goes in - there's
> > always the next merge window - I would prefer to take our time and have
> > it simmer in -next for max period.
> > 
> > So yeah, 2.23 has been tested for a long time now and it is very likely
> > that nothing would happen and if you think it's ok, then sure. Then if
> > you happen to see urgent pull requests with build or some other fixes,
> > at least you'll be prepared. :-)
> > 
> 
> This is just a documentation patch right? Nothing actually changes with
> the build. The only possible thing that we would potentially have to
> deal with is
> 
> (1) people noticing the doc change and complaining that they
> still need to use binutils-2.21 for some reason -- but they can't
> currently build an x86 kernel without patches anyway, so...

The __end_of_kernel_reserve symbol that breaks with the 2.21-2.22
binutils was added in v5.3, so we've already gone 3 kernel versions
without complaints.

> 
> (2) people noticing the doc change and suggesting moving to 2.26 or some
> later version instead of 2.23.

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Linus Torvalds]

On Tue, Mar 24, 2020 at 2:42 PM Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> This is just a documentation patch right?

That patch, yes.

However, there's a second patch that knows that if we have binutils >=
2.22, then we don't need to check for AVX2 or ADX support, because we
know it's there.

           Linus

Re: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[Arvind Sankar]

On Tue, Mar 24, 2020 at 03:14:38PM -0700, Linus Torvalds wrote:
> On Tue, Mar 24, 2020 at 2:42 PM Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > This is just a documentation patch right?
> 
> That patch, yes.
> 
> However, there's a second patch that knows that if we have binutils >=
> 2.22, then we don't need to check for AVX2 or ADX support, because we
> know it's there.
> 
>            Linus

Ok. But if you're otherwise ok taking those AVX2/ADX patches for 5.7, as
noted in my other email [1], the x86 build has been broken with
binutils <= 2.22 since v5.3.

[1] https://lore.kernel.org/lkml/20200324220147.GA3253486@rani.riverdale.lan/

RE: [PATCH] Documentation/changes: Raise minimum supported binutils version to 2.23

[David Laight]

From: Jason A. Donenfeld
> Sent: 23 March 2020 21:12
...
> By the way, while we're in the process of updating dependencies, what
> if we ratched the minimum binutils on x86 up to 2.25 (which is still
> quite old)? In this case, we could get rid of *all* of the CONFIG_AS_*
> ifdefs throughout.

Even my system has 2.23.2, although I've had to pull in a later
version of old of the elf libraries at some point.

Unfortunately we have to support customers who insist on using
very old distributions - and it is easiest to build things on
systems with similar old userspace.
The memcpy/memmove fiasco and changes to C++ 'character traits'
make anything else almost impossible.

Unnecessary minimum versions do cause grief.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)