diff mbox series

[5.15,v2,1/5] arch: fix broken BuildID for arm64 and riscv

Message ID 20230210-tsaeger-upstream-linux-stable-5-15-v2-1-6c68622745e9@oracle.com (mailing list archive)
State New, archived
Headers show
Series Fix Build ID on arm64 if CONFIG_MODVERSIONS=y | expand

Commit Message

Tom Saeger Feb. 10, 2023, 8:18 p.m. UTC 
From: Masahiro Yamada <masahiroy@kernel.org>

commit 99cb0d917ffa1ab628bb67364ca9b162c07699b1 upstream.

Dennis Gilmore reports that the BuildID is missing in the arm64 vmlinux
since commit 994b7ac1697b ("arm64: remove special treatment for the
link order of head.o").

The issue is that the type of .notes section, which contains the BuildID,
changed from NOTES to PROGBITS.

Ard Biesheuvel figured out that whichever object gets linked first gets
to decide the type of a section. The PROGBITS type is the result of the
compiler emitting .note.GNU-stack as PROGBITS rather than NOTE.

While Ard provided a fix for arm64, I want to fix this globally because
the same issue is happening on riscv since commit 2348e6bf4421 ("riscv:
remove special treatment for the link order of head.o"). This problem
will happen in general for other architectures if they start to drop
unneeded entries from scripts/head-object-list.txt.

Discard .note.GNU-stack in include/asm-generic/vmlinux.lds.h.

Link: https://lore.kernel.org/lkml/CAABkxwuQoz1CTbyb57n0ZX65eSYiTonFCU8-LCQc=74D=xE=rA@mail.gmail.com/
Fixes: 994b7ac1697b ("arm64: remove special treatment for the link order of head.o")
Fixes: 2348e6bf4421 ("riscv: remove special treatment for the link order of head.o")
Reported-by: Dennis Gilmore <dennis@ausil.us>
Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
Signed-off-by: Tom Saeger <tom.saeger@oracle.com>
---
 include/asm-generic/vmlinux.lds.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Greg Kroah-Hartman Feb. 23, 2023, 9:51 a.m. UTC | #1 
On Fri, Feb 10, 2023 at 01:18:40PM -0700, Tom Saeger wrote:
> From: Masahiro Yamada <masahiroy@kernel.org>
> 
> commit 99cb0d917ffa1ab628bb67364ca9b162c07699b1 upstream.
> 
> Dennis Gilmore reports that the BuildID is missing in the arm64 vmlinux
> since commit 994b7ac1697b ("arm64: remove special treatment for the
> link order of head.o").
> 
> The issue is that the type of .notes section, which contains the BuildID,
> changed from NOTES to PROGBITS.
> 
> Ard Biesheuvel figured out that whichever object gets linked first gets
> to decide the type of a section. The PROGBITS type is the result of the
> compiler emitting .note.GNU-stack as PROGBITS rather than NOTE.
> 
> While Ard provided a fix for arm64, I want to fix this globally because
> the same issue is happening on riscv since commit 2348e6bf4421 ("riscv:
> remove special treatment for the link order of head.o"). This problem
> will happen in general for other architectures if they start to drop
> unneeded entries from scripts/head-object-list.txt.
> 
> Discard .note.GNU-stack in include/asm-generic/vmlinux.lds.h.
> 
> Link: https://lore.kernel.org/lkml/CAABkxwuQoz1CTbyb57n0ZX65eSYiTonFCU8-LCQc=74D=xE=rA@mail.gmail.com/
> Fixes: 994b7ac1697b ("arm64: remove special treatment for the link order of head.o")
> Fixes: 2348e6bf4421 ("riscv: remove special treatment for the link order of head.o")

Why are we adding a commit to 5.15.y that fixes an issue that only
showed up in 6.1.y?

We need a good comment somewhere saying why this is needed...

thanks,

greg k-h
Tom Saeger Feb. 23, 2023, 5:53 p.m. UTC | #2 
On Thu, Feb 23, 2023 at 10:51:45AM +0100, Greg Kroah-Hartman wrote:
> On Fri, Feb 10, 2023 at 01:18:40PM -0700, Tom Saeger wrote:
> > From: Masahiro Yamada <masahiroy@kernel.org>
> > 
> > commit 99cb0d917ffa1ab628bb67364ca9b162c07699b1 upstream.
> > 
> > Dennis Gilmore reports that the BuildID is missing in the arm64 vmlinux
> > since commit 994b7ac1697b ("arm64: remove special treatment for the
> > link order of head.o").
> > 
> > The issue is that the type of .notes section, which contains the BuildID,
> > changed from NOTES to PROGBITS.
> > 
> > Ard Biesheuvel figured out that whichever object gets linked first gets
> > to decide the type of a section. The PROGBITS type is the result of the
> > compiler emitting .note.GNU-stack as PROGBITS rather than NOTE.
> > 
> > While Ard provided a fix for arm64, I want to fix this globally because
> > the same issue is happening on riscv since commit 2348e6bf4421 ("riscv:
> > remove special treatment for the link order of head.o"). This problem
> > will happen in general for other architectures if they start to drop
> > unneeded entries from scripts/head-object-list.txt.
> > 
> > Discard .note.GNU-stack in include/asm-generic/vmlinux.lds.h.
> > 
> > Link: https://lore.kernel.org/lkml/CAABkxwuQoz1CTbyb57n0ZX65eSYiTonFCU8-LCQc=74D=xE=rA@mail.gmail.com/
> > Fixes: 994b7ac1697b ("arm64: remove special treatment for the link order of head.o")
> > Fixes: 2348e6bf4421 ("riscv: remove special treatment for the link order of head.o")
> 
> Why are we adding a commit to 5.15.y that fixes an issue that only
> showed up in 6.1.y?

Only in 6.1.y?  No, not true. It was just the
observed manifestation of 'ld' quirkiness at that time in mainline.

This same issue "missing Build ID in arm64 vmlinux"
also exists in stable with CONFIG_MODVERSIONS=y arm64 since:
5.15.60+
5.10.136+
5.4.210+

These all had backports of:
0d362be5b142 ("Makefile: link with -z noexecstack --no-warn-rwx-segments")
which with CONFIG_MODVERSIONS=y brought about an observable 'ld' quirkiness.

Both are related to a behavior change in different versions of binutils ld and the
kernel's linker script.

99cb0d917ffa ("arch: fix broken BuildID for arm64 and riscv")
IS the mechanism which works-around the ld quirkiness, by adjusting
kernel's linker script.

a494398bde27 ("s390: define RUNTIME_DISCARD_EXIT to fix link error with GNU ld < 2.36")
Documents the binutils commit which changed ld's behavior.

The entire sequence (dependencies and fixes) IS the 5.4 patch series I sent.
It provides the kernel linker script mechanism and architecture hooks to
work with 'ld' versions before and after...

5.10, and 5.15 are similar, but already had dependency patches.

Please reconsider applying the 5.15, 5.10, and 5.4 series, as they fix a
real problem.

> 
> We need a good comment somewhere saying why this is needed...

Does the above suffice?

> 
> thanks,
> 
> greg k-h
Tom Saeger Feb. 24, 2023, 2:47 a.m. UTC | #3 
On Thu, Feb 23, 2023 at 11:53:42AM -0600, Tom Saeger wrote:
> On Thu, Feb 23, 2023 at 10:51:45AM +0100, Greg Kroah-Hartman wrote:
> > On Fri, Feb 10, 2023 at 01:18:40PM -0700, Tom Saeger wrote:
> > > From: Masahiro Yamada <masahiroy@kernel.org>
> > > 
> > > commit 99cb0d917ffa1ab628bb67364ca9b162c07699b1 upstream.
> > > 
> > > Dennis Gilmore reports that the BuildID is missing in the arm64 vmlinux
> > > since commit 994b7ac1697b ("arm64: remove special treatment for the
> > > link order of head.o").
> > > 
> > > The issue is that the type of .notes section, which contains the BuildID,
> > > changed from NOTES to PROGBITS.
> > > 
> > > Ard Biesheuvel figured out that whichever object gets linked first gets
> > > to decide the type of a section. The PROGBITS type is the result of the
> > > compiler emitting .note.GNU-stack as PROGBITS rather than NOTE.
> > > 
> > > While Ard provided a fix for arm64, I want to fix this globally because
> > > the same issue is happening on riscv since commit 2348e6bf4421 ("riscv:
> > > remove special treatment for the link order of head.o"). This problem
> > > will happen in general for other architectures if they start to drop
> > > unneeded entries from scripts/head-object-list.txt.
> > > 
> > > Discard .note.GNU-stack in include/asm-generic/vmlinux.lds.h.
> > > 
> > > Link: https://lore.kernel.org/lkml/CAABkxwuQoz1CTbyb57n0ZX65eSYiTonFCU8-LCQc=74D=xE=rA@mail.gmail.com/
> > > Fixes: 994b7ac1697b ("arm64: remove special treatment for the link order of head.o")
> > > Fixes: 2348e6bf4421 ("riscv: remove special treatment for the link order of head.o")

Greg, how about something like this tacked onto backport of this commit?

[Tom: stable backport 5.15.y, 5.10.y, 5.4.y]

Though the above "Fixes:" commits are not in this kernel, the conditions
which lead to a missing Build ID in arm64 vmlinux are similar.

Evidence points to these conditions:
1. ld version > 2.36 (exact binutils commit documented in a494398bde27)
2. first object which gets linked (head.o) has a PROGBITS .note.GNU-stack segment

These conditions can be observed when:
- 5.15.60+ OR 5.10.136+ OR 5.4.210+
- AND ld version > 2.36
- AND arch=arm64
- AND CONFIG_MODVERSIONS=y

This was previously bisected to the stable backport of 0d362be5b142.
Follow-up experiments were discussed here: https://lore.kernel.org/all/20221221235413.xaisboqmr7dkqwn6@oracle.com/ 
which strongly hints at condition 2.


> > 
> > Why are we adding a commit to 5.15.y that fixes an issue that only
> > showed up in 6.1.y?

If you approve - I'll send v3 for 5.15, 5.10, and 5.4 (with style fixes).

Cheers,

--Tom
Greg Kroah-Hartman Feb. 28, 2023, 8:58 a.m. UTC | #4 
On Thu, Feb 23, 2023 at 08:47:24PM -0600, Tom Saeger wrote:
> On Thu, Feb 23, 2023 at 11:53:42AM -0600, Tom Saeger wrote:
> > On Thu, Feb 23, 2023 at 10:51:45AM +0100, Greg Kroah-Hartman wrote:
> > > On Fri, Feb 10, 2023 at 01:18:40PM -0700, Tom Saeger wrote:
> > > > From: Masahiro Yamada <masahiroy@kernel.org>
> > > > 
> > > > commit 99cb0d917ffa1ab628bb67364ca9b162c07699b1 upstream.
> > > > 
> > > > Dennis Gilmore reports that the BuildID is missing in the arm64 vmlinux
> > > > since commit 994b7ac1697b ("arm64: remove special treatment for the
> > > > link order of head.o").
> > > > 
> > > > The issue is that the type of .notes section, which contains the BuildID,
> > > > changed from NOTES to PROGBITS.
> > > > 
> > > > Ard Biesheuvel figured out that whichever object gets linked first gets
> > > > to decide the type of a section. The PROGBITS type is the result of the
> > > > compiler emitting .note.GNU-stack as PROGBITS rather than NOTE.
> > > > 
> > > > While Ard provided a fix for arm64, I want to fix this globally because
> > > > the same issue is happening on riscv since commit 2348e6bf4421 ("riscv:
> > > > remove special treatment for the link order of head.o"). This problem
> > > > will happen in general for other architectures if they start to drop
> > > > unneeded entries from scripts/head-object-list.txt.
> > > > 
> > > > Discard .note.GNU-stack in include/asm-generic/vmlinux.lds.h.
> > > > 
> > > > Link: https://lore.kernel.org/lkml/CAABkxwuQoz1CTbyb57n0ZX65eSYiTonFCU8-LCQc=74D=xE=rA@mail.gmail.com/
> > > > Fixes: 994b7ac1697b ("arm64: remove special treatment for the link order of head.o")
> > > > Fixes: 2348e6bf4421 ("riscv: remove special treatment for the link order of head.o")
> 
> Greg, how about something like this tacked onto backport of this commit?
> 
> [Tom: stable backport 5.15.y, 5.10.y, 5.4.y]
> 
> Though the above "Fixes:" commits are not in this kernel, the conditions
> which lead to a missing Build ID in arm64 vmlinux are similar.
> 
> Evidence points to these conditions:
> 1. ld version > 2.36 (exact binutils commit documented in a494398bde27)
> 2. first object which gets linked (head.o) has a PROGBITS .note.GNU-stack segment
> 
> These conditions can be observed when:
> - 5.15.60+ OR 5.10.136+ OR 5.4.210+
> - AND ld version > 2.36
> - AND arch=arm64
> - AND CONFIG_MODVERSIONS=y
> 
> This was previously bisected to the stable backport of 0d362be5b142.
> Follow-up experiments were discussed here: https://lore.kernel.org/all/20221221235413.xaisboqmr7dkqwn6@oracle.com/ 
> which strongly hints at condition 2.
> 
> 
> > > 
> > > Why are we adding a commit to 5.15.y that fixes an issue that only
> > > showed up in 6.1.y?
> 
> If you approve - I'll send v3 for 5.15, 5.10, and 5.4 (with style fixes).

That would make more sense, thanks.

greg k-h
diff mbox series

Patch

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index e28792ca25a1..8471717c5085 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -903,7 +903,12 @@ 
 #define PRINTK_INDEX
 #endif
 
+/*
+ * Discard .note.GNU-stack, which is emitted as PROGBITS by the compiler.
+ * Otherwise, the type of .notes section would become PROGBITS instead of NOTES.
+ */
 #define NOTES								\
+	/DISCARD/ : { *(.note.GNU-stack) }				\
 	.notes : AT(ADDR(.notes) - LOAD_OFFSET) {			\
 		__start_notes = .;					\
 		KEEP(*(.note.*))					\