Tom, thanks for pursuing this. Sorry I'm falling behind in reviews
(going offline soon for the holidays). Some additional questions:
On Thu, Dec 15, 2022 at 3:18 PM Tom Saeger <tom.saeger@xxxxxxxxxx> wrote:
>
> Backport of:
> commit 0d362be5b142 ("Makefile: link with -z noexecstack --no-warn-rwx-segments")
> breaks arm64 Build ID when CONFIG_MODVERSIONS=y for all kernels
> from: commit e4484a495586 ("Merge tag 'kbuild-fixes-v5.0' of git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild")
> until: commit df202b452fe6 ("Merge tag 'kbuild-v5.19' of git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild")
Just arm64? Why not other architectures?
>
> Linus's tree doesn't have this issue since 0d362be5b142 was merged
> after df202b452fe6 which included:
> commit 7b4537199a4a ("kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS")
>
> This kernel's KBUILD CONFIG_MODVERSIONS tooling compiles and links .S targets
> with relocatable (-r) and now (-z noexecstack)
> which results in ld adding a .note.GNU-stack section to .o files.
> Final linking of vmlinux should add a .NOTES segment containing the
> Build ID, but does NOT (on some architectures like arm64) if a
> .note.GNU-stack section is found in .o's supplied during link
> of vmlinux.
Is that a bug in BFD? That the behavior differs per target
architecture is subtle. If it's not documented behavior that you can
link to, can you file a bug about your findings and cc me?
https://sourceware.org/bugzilla/enter_bug.cgi?product=binutils
If it is a bug in BFD, then I'm not opposed to working around it, but
it would be good to have as precise a report as possible in the commit
message if we're going to do hijinks in a stable-only patch for
existing tooling.
If it's a feature, having some explanation _why_ we get per-arch
behavior like this may be helpful for us to link to in the future
should this come up again.
>
> DISCARD .note.GNU-stack sections of .S targets. Final link of
That's going to give them an executable stack again.
https://www.redhat.com/en/blog/linkers-warnings-about-executable-stacks-and-segments
>> missing .note.GNU-stack section implies executable stack
The intent of 0d362be5b142 is that we don't want translation units to
have executable stacks, though I do note that assembler sources need
to opt in.
Is it possible to force a build-id via linker flag `--build-id=sha1`?
If not, can we just use `-z execstack` rather than concatenating a
DISCARD section into a linker script? Either command line flags feel
cleaner than modifying a linker script at build time, if they work
that is.
> vmlinux then properly adds .NOTES segment containing Build ID that can
> be read using tools like 'readelf -n'.
>
> Fixes: 0d362be5b142 ("Makefile: link with -z noexecstack --no-warn-rwx-segments")
> Cc: <stable@xxxxxxxxxxxxxxx> # 5.15, 5.10, 5.4
> Cc: <linux-kbuild@xxxxxxxxxxxxxxx>
> Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> Cc: Nicholas Piggin <npiggin@xxxxxxxxx>
> Cc: Michal Marek <michal.lkml@xxxxxxxxxxx>
> Cc: Nathan Chancellor <nathan@xxxxxxxxxx>
> Signed-off-by: Tom Saeger <tom.saeger@xxxxxxxxxx>
> ---
>
> v2:
> - Changed approach to append DISCARD section to generated linker script.
> - ld no longer emits warning (which was intent of 0d362b35b142) this
> addresses Nick's v1 feedback.
> - this is applied to all arches, not just arm64
> - added commit refs and notes why this doesn't occur in Linus's tree
> to address Greg's v1 feedback.
> - added Fixes: 0d362b35b142 requested by Nick
> - added note to changelog for 7b4537199a4a requested by Nick
> - build tested on arm64 and x86
>
> version works(vmlinux contains Build ID)
> v4.14.302 x86, arm64
> v4.14.302.patched x86, arm64
> v4.19.269 x86, arm64
> v4.19.269.patched x86, arm64
> v5.4.227 x86
> v5.4.227.patched x86, arm64
> v5.10.159 x86
> v5.10.159.patched x86, arm64
> v5.15.83 x86
> v5.15.83.patched x86, arm64
>
> v1: https://lore.kernel.org/all/cover.1670358255.git.tom.saeger@xxxxxxxxxx/
>
>
> scripts/Makefile.build | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 17aa8ef2d52a..e3939676eeb5 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -379,6 +379,8 @@ cmd_modversions_S = \
> if $(OBJDUMP) -h $@ | grep -q __ksymtab; then \
> $(call cmd_gensymtypes_S,$(KBUILD_SYMTYPES),$(@:.o=.symtypes)) \
> > $(@D)/.tmp_$(@F:.o=.ver); \
> + echo "SECTIONS { /DISCARD/ : { *(.note.GNU-stack) } }" \
> + >> $(@D)/.tmp_$(@F:.o=.ver); \
> \
> $(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@ \
> -T $(@D)/.tmp_$(@F:.o=.ver); \
>
> base-commit: fd6d66840b4269da4e90e1ea807ae3197433bc66
> --
> 2.38.1
>
--
Thanks,
~Nick Desaulniers
