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")
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.
DISCARD .note.GNU-stack sections of .S targets. Final link of 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@vger.kernel.org # 5.15, 5.10, 5.4 Cc: linux-kbuild@vger.kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Masahiro Yamada masahiroy@kernel.org Cc: Nicholas Piggin npiggin@gmail.com Cc: Michal Marek michal.lkml@markovi.net Cc: Nathan Chancellor nathan@kernel.org Signed-off-by: Tom Saeger tom.saeger@oracle.com ---
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@oracle.com/
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
On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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")
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")
Why can't we add this one instead of a custom change?
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.
DISCARD .note.GNU-stack sections of .S targets. Final link of 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@vger.kernel.org # 5.15, 5.10, 5.4 Cc: linux-kbuild@vger.kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Masahiro Yamada masahiroy@kernel.org Cc: Nicholas Piggin npiggin@gmail.com Cc: Michal Marek michal.lkml@markovi.net Cc: Nathan Chancellor nathan@kernel.org Signed-off-by: Tom Saeger tom.saeger@oracle.com
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@oracle.com/
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); \ \base-commit: fd6d66840b4269da4e90e1ea807ae3197433bc66
2.38.1
I need some acks from some developers/maintainers before I can take this... {hint}
thanks,
greg k-h
On Wed, Dec 21, 2022 at 05:31:51PM +0100, Greg Kroah-Hartman wrote:
On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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")
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")
Why can't we add this one instead of a custom change?
I quickly abandoned that route - there are too many dependencies.
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.
DISCARD .note.GNU-stack sections of .S targets. Final link of 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@vger.kernel.org # 5.15, 5.10, 5.4 Cc: linux-kbuild@vger.kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Masahiro Yamada masahiroy@kernel.org Cc: Nicholas Piggin npiggin@gmail.com Cc: Michal Marek michal.lkml@markovi.net Cc: Nathan Chancellor nathan@kernel.org Signed-off-by: Tom Saeger tom.saeger@oracle.com
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@oracle.com/
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); \ \base-commit: fd6d66840b4269da4e90e1ea807ae3197433bc66
2.38.1
I need some acks from some developers/maintainers before I can take this... {hint}
thanks,
greg k-h
On Wed, Dec 21, 2022 at 02:52:10PM -0600, Tom Saeger wrote:
On Wed, Dec 21, 2022 at 05:31:51PM +0100, Greg Kroah-Hartman wrote:
On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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")
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")
Why can't we add this one instead of a custom change?
I quickly abandoned that route - there are too many dependencies.
How many? Why? Whenever we add a "this is not upstream" patch, 90% of the time it is incorrect and causes problems (merge issues included.) So please please please let's try to keep in sync with what is in Linus's tree.
thanks,
greg k-h
On Thu, Dec 22, 2022 at 07:01:11AM +0100, Greg Kroah-Hartman wrote:
On Wed, Dec 21, 2022 at 02:52:10PM -0600, Tom Saeger wrote:
On Wed, Dec 21, 2022 at 05:31:51PM +0100, Greg Kroah-Hartman wrote:
On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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")
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")
Why can't we add this one instead of a custom change?
I quickly abandoned that route - there are too many dependencies.
How many? Why? Whenever we add a "this is not upstream" patch, 90% of the time it is incorrect and causes problems (merge issues included.) So please please please let's try to keep in sync with what is in Linus's tree.
thanks,
greg k-h
Ok - I spent some time on this.
The haystack I searched:
git rev-list --grep="masahiroy/linux-kbuild" v5.15..v5.19-rc1 | while read -r CID ; do git rev-list "${CID}^-" ; done | wc -l 182
I have 54 of those 182 applied to 5.15.85, and this works in my limited build testing (x86_64 gcc, arm64 gcc, arm64 clang).
Specifically:
cbfc9bf3223f genksyms: adjust the output format to modpost e7c9c2630e59 kbuild: stop merging *.symversions 1d788aa800c7 kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS 8a01c770955b modpost: extract symbol versions from *.cmd files a8ade6b33772 modpost: add sym_find_with_module() helper a9639fe6b516 modpost: change the license of EXPORT_SYMBOL to bool type 04804878f631 modpost: remove left-over cross_compile declaration 3388b8af9698 kbuild: record symbol versions in *.cmd files 4ff3946463a0 kbuild: generate a list of objects in vmlinux 074617e2ad6a modpost: move *.mod.c generation to write_mod_c_files() 81b78cb6e821 modpost: merge add_{intree_flag,retpoline,staging_flag} to add_header 9df4f00b53b4 modpost: split new_symbol() to symbol allocation and hash table addition 85728bcbc500 modpost: make sym_add_exported() always allocate a new symbol 82aa2b4d30af modpost: make multiple export error 6cc962f0a175 modpost: dump Module.symvers in the same order of modules.order 39db82cea373 modpost: traverse the namespace_list in order 45dc7b236dcb modpost: use doubly linked list for dump_lists 2a322506403a modpost: traverse unresolved symbols in order a85718443348 modpost: add sym_add_unresolved() helper 5c44b0f89c82 modpost: traverse modules in order a0b68f6655f2 modpost: import include/linux/list.h ce9f4d32be4e modpost: change mod->gpl_compatible to bool type f9fe36a515ca modpost: use bool type where appropriate 46f6334d7055 modpost: move struct namespace_list to modpost.c afa24c45af49 modpost: retrieve the module dependency and CRCs in check_exports() a8f687dc3ac2 modpost: add a separate error for exported symbols without definition f97f0e32b230 modpost: remove stale comment about sym_add_exported() 0af2ad9d11c3 modpost: do not write out any file when error occurred 09eac5681c02 modpost: use snprintf() instead of sprintf() for safety ee07380110f2 kbuild: read *.mod to get objects passed to $(LD) or $(AR) 97976e5c6d55 kbuild: make *.mod not depend on *.o 0d4368c8da07 kbuild: get rid of duplication in *.mod files 55f602f00903 kbuild: split the second line of *.mod into *.usyms ea9730eb0788 kbuild: reuse real-search to simplify cmd_mod 1eacf71f885a kbuild: make multi_depend work with targets in subdirectory 19c2b5b6f769 kbuild: reuse suffix-search to refactor multi_depend 75df07a9133d kbuild: refactor cmd_modversions_S 53257fbea174 kbuild: refactor cmd_modversions_c b6e50682c261 modpost: remove annoying namespace_from_kstrtabns() 1002d8f060b0 modpost: remove redundant initializes for static variables 921fbb7ab714 modpost: move export_from_secname() call to more relevant place f49c0989e01b modpost: remove useless export_from_sec() 7a98501a77db kbuild: do not remove empty *.symtypes explicitly 500f1b31c16f kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} e04fcad29aa3 kallsyms: ignore all local labels prefixed by '.L' 9e01f7ef15d2 kbuild: drop $(size_append) from cmd_zstd 054133567480 kbuild: do not include include/config/auto.conf from shell scripts 34d14831eecb kbuild: stop using config_filename in scripts/Makefile.modsign 75155bda5498 kbuild: use more subdir- for visiting subdirectories while cleaning 1a3f00cd3be8 kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules 47704d10e997 kbuild: detect objtool update without using .SECONDEXPANSION 7a89d034ccc6 kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro 3cbbf4b9d188 kbuild: store the objtool command in *.cmd files 467f0d0aa6b4 kbuild: rename __objtool_obj and reuse it for cmd_cc_lto_link_modules
There may be a few more patches post v5.19-rc1 for Fixes. I haven't tried minimizing the 54.
Greg - is 54 too many?
Regards,
--Tom
On Tue, Jan 10, 2023 at 3:36 AM Tom Saeger tom.saeger@oracle.com wrote:
On Thu, Dec 22, 2022 at 07:01:11AM +0100, Greg Kroah-Hartman wrote:
On Wed, Dec 21, 2022 at 02:52:10PM -0600, Tom Saeger wrote:
On Wed, Dec 21, 2022 at 05:31:51PM +0100, Greg Kroah-Hartman wrote:
On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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")
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")
Why can't we add this one instead of a custom change?
I quickly abandoned that route - there are too many dependencies.
How many? Why? Whenever we add a "this is not upstream" patch, 90% of the time it is incorrect and causes problems (merge issues included.) So please please please let's try to keep in sync with what is in Linus's tree.
thanks,
greg k-h
Ok - I spent some time on this.
The haystack I searched:
git rev-list --grep="masahiroy/linux-kbuild" v5.15..v5.19-rc1 | while read -r CID ; do git rev-list "${CID}^-" ; done | wc -l 182
I have 54 of those 182 applied to 5.15.85, and this works in my limited build testing (x86_64 gcc, arm64 gcc, arm64 clang).
Specifically:
cbfc9bf3223f genksyms: adjust the output format to modpost e7c9c2630e59 kbuild: stop merging *.symversions 1d788aa800c7 kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS 8a01c770955b modpost: extract symbol versions from *.cmd files a8ade6b33772 modpost: add sym_find_with_module() helper a9639fe6b516 modpost: change the license of EXPORT_SYMBOL to bool type 04804878f631 modpost: remove left-over cross_compile declaration 3388b8af9698 kbuild: record symbol versions in *.cmd files 4ff3946463a0 kbuild: generate a list of objects in vmlinux 074617e2ad6a modpost: move *.mod.c generation to write_mod_c_files() 81b78cb6e821 modpost: merge add_{intree_flag,retpoline,staging_flag} to add_header 9df4f00b53b4 modpost: split new_symbol() to symbol allocation and hash table addition 85728bcbc500 modpost: make sym_add_exported() always allocate a new symbol 82aa2b4d30af modpost: make multiple export error 6cc962f0a175 modpost: dump Module.symvers in the same order of modules.order 39db82cea373 modpost: traverse the namespace_list in order 45dc7b236dcb modpost: use doubly linked list for dump_lists 2a322506403a modpost: traverse unresolved symbols in order a85718443348 modpost: add sym_add_unresolved() helper 5c44b0f89c82 modpost: traverse modules in order a0b68f6655f2 modpost: import include/linux/list.h ce9f4d32be4e modpost: change mod->gpl_compatible to bool type f9fe36a515ca modpost: use bool type where appropriate 46f6334d7055 modpost: move struct namespace_list to modpost.c afa24c45af49 modpost: retrieve the module dependency and CRCs in check_exports() a8f687dc3ac2 modpost: add a separate error for exported symbols without definition f97f0e32b230 modpost: remove stale comment about sym_add_exported() 0af2ad9d11c3 modpost: do not write out any file when error occurred 09eac5681c02 modpost: use snprintf() instead of sprintf() for safety ee07380110f2 kbuild: read *.mod to get objects passed to $(LD) or $(AR) 97976e5c6d55 kbuild: make *.mod not depend on *.o 0d4368c8da07 kbuild: get rid of duplication in *.mod files 55f602f00903 kbuild: split the second line of *.mod into *.usyms ea9730eb0788 kbuild: reuse real-search to simplify cmd_mod 1eacf71f885a kbuild: make multi_depend work with targets in subdirectory 19c2b5b6f769 kbuild: reuse suffix-search to refactor multi_depend 75df07a9133d kbuild: refactor cmd_modversions_S 53257fbea174 kbuild: refactor cmd_modversions_c b6e50682c261 modpost: remove annoying namespace_from_kstrtabns() 1002d8f060b0 modpost: remove redundant initializes for static variables 921fbb7ab714 modpost: move export_from_secname() call to more relevant place f49c0989e01b modpost: remove useless export_from_sec() 7a98501a77db kbuild: do not remove empty *.symtypes explicitly 500f1b31c16f kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} e04fcad29aa3 kallsyms: ignore all local labels prefixed by '.L' 9e01f7ef15d2 kbuild: drop $(size_append) from cmd_zstd 054133567480 kbuild: do not include include/config/auto.conf from shell scripts 34d14831eecb kbuild: stop using config_filename in scripts/Makefile.modsign 75155bda5498 kbuild: use more subdir- for visiting subdirectories while cleaning 1a3f00cd3be8 kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules 47704d10e997 kbuild: detect objtool update without using .SECONDEXPANSION 7a89d034ccc6 kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro 3cbbf4b9d188 kbuild: store the objtool command in *.cmd files 467f0d0aa6b4 kbuild: rename __objtool_obj and reuse it for cmd_cc_lto_link_modules
There may be a few more patches post v5.19-rc1 for Fixes. I haven't tried minimizing the 54.
It is up to Greg.
Indeed, 7b4537199a4a requires a lot of prerequisite commits. I do not remember which ones are exactly mandatory.
Greg - is 54 too many?
Regards,
--Tom
On Tue, Jan 10, 2023 at 03:58:11PM +0900, Masahiro Yamada wrote:
On Tue, Jan 10, 2023 at 3:36 AM Tom Saeger tom.saeger@oracle.com wrote:
On Thu, Dec 22, 2022 at 07:01:11AM +0100, Greg Kroah-Hartman wrote:
On Wed, Dec 21, 2022 at 02:52:10PM -0600, Tom Saeger wrote:
On Wed, Dec 21, 2022 at 05:31:51PM +0100, Greg Kroah-Hartman wrote:
On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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")
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")
Why can't we add this one instead of a custom change?
I quickly abandoned that route - there are too many dependencies.
How many? Why? Whenever we add a "this is not upstream" patch, 90% of the time it is incorrect and causes problems (merge issues included.) So please please please let's try to keep in sync with what is in Linus's tree.
thanks,
greg k-h
Ok - I spent some time on this.
The haystack I searched:
git rev-list --grep="masahiroy/linux-kbuild" v5.15..v5.19-rc1 | while read -r CID ; do git rev-list "${CID}^-" ; done | wc -l 182
I have 54 of those 182 applied to 5.15.85, and this works in my limited build testing (x86_64 gcc, arm64 gcc, arm64 clang).
Specifically:
It probably makes more sense to post the cherry-picked hashes:
5ce2176b81f7 genksyms: adjust the output format to modpost 7375cbcf2343 kbuild: stop merging *.symversions 7b4537199a4a kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS f292d875d0dc modpost: extract symbol versions from *.cmd files 69c4cc99bbcb modpost: add sym_find_with_module() helper 2a66c3124afd modpost: change the license of EXPORT_SYMBOL to bool type ce79c406a24c modpost: remove left-over cross_compile declaration 78e9e56af385 kbuild: record symbol versions in *.cmd files e493f4727520 kbuild: generate a list of objects in vmlinux a44abaca0e19 modpost: move *.mod.c generation to write_mod_c_files() 7fedac9698b3 modpost: merge add_{intree_flag,retpoline,staging_flag} to add_header f18379a30271 modpost: split new_symbol() to symbol allocation and hash table addition e76cc48d8e6d modpost: make sym_add_exported() always allocate a new symbol b8422711080f modpost: make multiple export error f841536e8c5b modpost: dump Module.symvers in the same order of modules.order ab489d6002fc modpost: traverse the namespace_list in order 4484054816ca modpost: use doubly linked list for dump_lists 8a69152be9a8 modpost: traverse unresolved symbols in order e882e89bcf1d modpost: add sym_add_unresolved() helper 325eba05e8ab modpost: traverse modules in order 97aa4aef532a modpost: import include/linux/list.h 5066743e4c2f modpost: change mod->gpl_compatible to bool type 58e01fcae18c modpost: use bool type where appropriate 70ddb48db4aa modpost: move struct namespace_list to modpost.c 4cae77ac582b modpost: retrieve the module dependency and CRCs in check_exports() 23beb44a0eff modpost: add a separate error for exported symbols without definition 594ade3eef3f modpost: remove stale comment about sym_add_exported() c155a47d83ab modpost: do not write out any file when error occurred 15a28c7c7291 modpost: use snprintf() instead of sprintf() for safety feb7d79fea1d kbuild: read *.mod to get objects passed to $(LD) or $(AR) fc93a4cdce1d kbuild: make *.mod not depend on *.o 22f26f21774f kbuild: get rid of duplication in *.mod files 9413e7640564 kbuild: split the second line of *.mod into *.usyms b3591e061919 kbuild: reuse real-search to simplify cmd_mod f97cf399915b kbuild: make multi_depend work with targets in subdirectory 9eef99f7a335 kbuild: reuse suffix-search to refactor multi_depend 7cfa2fcbac16 kbuild: refactor cmd_modversions_S 8017ce50641c kbuild: refactor cmd_modversions_c 79f646e8654b modpost: remove annoying namespace_from_kstrtabns() b5f1a52a59eb modpost: remove redundant initializes for static variables 535b3e05f435 modpost: move export_from_secname() call to more relevant place 7ce3e410e018 modpost: remove useless export_from_sec() dc6dc3e7a73f kbuild: do not remove empty *.symtypes explicitly f43e31d5cb78 kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} d4c858643263 kallsyms: ignore all local labels prefixed by '.L' 64d8aaa4ef38 kbuild: drop $(size_append) from cmd_zstd 7d153696e5db kbuild: do not include include/config/auto.conf from shell scripts 4db9c2e3d055 kbuild: stop using config_filename in scripts/Makefile.modsign 8212f8986d31 kbuild: use more subdir- for visiting subdirectories while cleaning 90a353491e9f kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules ef62588c2c86 kbuild: detect objtool update without using .SECONDEXPANSION 918a6b7f6846 kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro 92594d569b6d kbuild: store the objtool command in *.cmd files 5c4859e77aa1 kbuild: rename __objtool_obj and reuse it for cmd_cc_lto_link_modules
There may be a few more patches post v5.19-rc1 for Fixes. I haven't tried minimizing the 54.
It is up to Greg.
Indeed, 7b4537199a4a requires a lot of prerequisite commits. I do not remember which ones are exactly mandatory.
Greg - is 54 too many?
Regards,
--Tom
-- Best Regards Masahiro Yamada
On Mon, Jan 09, 2023 at 12:36:15PM -0600, Tom Saeger wrote:
On Thu, Dec 22, 2022 at 07:01:11AM +0100, Greg Kroah-Hartman wrote:
On Wed, Dec 21, 2022 at 02:52:10PM -0600, Tom Saeger wrote:
On Wed, Dec 21, 2022 at 05:31:51PM +0100, Greg Kroah-Hartman wrote:
On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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")
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")
Why can't we add this one instead of a custom change?
I quickly abandoned that route - there are too many dependencies.
How many? Why? Whenever we add a "this is not upstream" patch, 90% of the time it is incorrect and causes problems (merge issues included.) So please please please let's try to keep in sync with what is in Linus's tree.
thanks,
greg k-h
Ok - I spent some time on this.
The haystack I searched:
git rev-list --grep="masahiroy/linux-kbuild" v5.15..v5.19-rc1 | while read -r CID ; do git rev-list "${CID}^-" ; done | wc -l 182
I have 54 of those 182 applied to 5.15.85, and this works in my limited build testing (x86_64 gcc, arm64 gcc, arm64 clang).
Specifically:
cbfc9bf3223f genksyms: adjust the output format to modpost e7c9c2630e59 kbuild: stop merging *.symversions 1d788aa800c7 kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS 8a01c770955b modpost: extract symbol versions from *.cmd files a8ade6b33772 modpost: add sym_find_with_module() helper a9639fe6b516 modpost: change the license of EXPORT_SYMBOL to bool type 04804878f631 modpost: remove left-over cross_compile declaration 3388b8af9698 kbuild: record symbol versions in *.cmd files 4ff3946463a0 kbuild: generate a list of objects in vmlinux 074617e2ad6a modpost: move *.mod.c generation to write_mod_c_files() 81b78cb6e821 modpost: merge add_{intree_flag,retpoline,staging_flag} to add_header 9df4f00b53b4 modpost: split new_symbol() to symbol allocation and hash table addition 85728bcbc500 modpost: make sym_add_exported() always allocate a new symbol 82aa2b4d30af modpost: make multiple export error 6cc962f0a175 modpost: dump Module.symvers in the same order of modules.order 39db82cea373 modpost: traverse the namespace_list in order 45dc7b236dcb modpost: use doubly linked list for dump_lists 2a322506403a modpost: traverse unresolved symbols in order a85718443348 modpost: add sym_add_unresolved() helper 5c44b0f89c82 modpost: traverse modules in order a0b68f6655f2 modpost: import include/linux/list.h ce9f4d32be4e modpost: change mod->gpl_compatible to bool type f9fe36a515ca modpost: use bool type where appropriate 46f6334d7055 modpost: move struct namespace_list to modpost.c afa24c45af49 modpost: retrieve the module dependency and CRCs in check_exports() a8f687dc3ac2 modpost: add a separate error for exported symbols without definition f97f0e32b230 modpost: remove stale comment about sym_add_exported() 0af2ad9d11c3 modpost: do not write out any file when error occurred 09eac5681c02 modpost: use snprintf() instead of sprintf() for safety ee07380110f2 kbuild: read *.mod to get objects passed to $(LD) or $(AR) 97976e5c6d55 kbuild: make *.mod not depend on *.o 0d4368c8da07 kbuild: get rid of duplication in *.mod files 55f602f00903 kbuild: split the second line of *.mod into *.usyms ea9730eb0788 kbuild: reuse real-search to simplify cmd_mod 1eacf71f885a kbuild: make multi_depend work with targets in subdirectory 19c2b5b6f769 kbuild: reuse suffix-search to refactor multi_depend 75df07a9133d kbuild: refactor cmd_modversions_S 53257fbea174 kbuild: refactor cmd_modversions_c b6e50682c261 modpost: remove annoying namespace_from_kstrtabns() 1002d8f060b0 modpost: remove redundant initializes for static variables 921fbb7ab714 modpost: move export_from_secname() call to more relevant place f49c0989e01b modpost: remove useless export_from_sec() 7a98501a77db kbuild: do not remove empty *.symtypes explicitly 500f1b31c16f kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} e04fcad29aa3 kallsyms: ignore all local labels prefixed by '.L' 9e01f7ef15d2 kbuild: drop $(size_append) from cmd_zstd 054133567480 kbuild: do not include include/config/auto.conf from shell scripts 34d14831eecb kbuild: stop using config_filename in scripts/Makefile.modsign 75155bda5498 kbuild: use more subdir- for visiting subdirectories while cleaning 1a3f00cd3be8 kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules 47704d10e997 kbuild: detect objtool update without using .SECONDEXPANSION 7a89d034ccc6 kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro 3cbbf4b9d188 kbuild: store the objtool command in *.cmd files 467f0d0aa6b4 kbuild: rename __objtool_obj and reuse it for cmd_cc_lto_link_modules
There may be a few more patches post v5.19-rc1 for Fixes. I haven't tried minimizing the 54.
Greg - is 54 too many?
Yes.
How about we just revert the original problem commit here to solve this mess? Wouldn't that be easier overall?
thanks,
greg k-h
On Thu, Jan 12, 2023 at 01:00:57PM +0100, Greg Kroah-Hartman wrote:
On Mon, Jan 09, 2023 at 12:36:15PM -0600, Tom Saeger wrote:
On Thu, Dec 22, 2022 at 07:01:11AM +0100, Greg Kroah-Hartman wrote:
On Wed, Dec 21, 2022 at 02:52:10PM -0600, Tom Saeger wrote:
On Wed, Dec 21, 2022 at 05:31:51PM +0100, Greg Kroah-Hartman wrote:
On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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")
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")
Why can't we add this one instead of a custom change?
I quickly abandoned that route - there are too many dependencies.
How many? Why? Whenever we add a "this is not upstream" patch, 90% of the time it is incorrect and causes problems (merge issues included.) So please please please let's try to keep in sync with what is in Linus's tree.
thanks,
greg k-h
Ok - I spent some time on this.
The haystack I searched:
git rev-list --grep="masahiroy/linux-kbuild" v5.15..v5.19-rc1 | while read -r CID ; do git rev-list "${CID}^-" ; done | wc -l 182
I have 54 of those 182 applied to 5.15.85, and this works in my limited build testing (x86_64 gcc, arm64 gcc, arm64 clang).
Specifically:
cbfc9bf3223f genksyms: adjust the output format to modpost e7c9c2630e59 kbuild: stop merging *.symversions 1d788aa800c7 kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS 8a01c770955b modpost: extract symbol versions from *.cmd files a8ade6b33772 modpost: add sym_find_with_module() helper a9639fe6b516 modpost: change the license of EXPORT_SYMBOL to bool type 04804878f631 modpost: remove left-over cross_compile declaration 3388b8af9698 kbuild: record symbol versions in *.cmd files 4ff3946463a0 kbuild: generate a list of objects in vmlinux 074617e2ad6a modpost: move *.mod.c generation to write_mod_c_files() 81b78cb6e821 modpost: merge add_{intree_flag,retpoline,staging_flag} to add_header 9df4f00b53b4 modpost: split new_symbol() to symbol allocation and hash table addition 85728bcbc500 modpost: make sym_add_exported() always allocate a new symbol 82aa2b4d30af modpost: make multiple export error 6cc962f0a175 modpost: dump Module.symvers in the same order of modules.order 39db82cea373 modpost: traverse the namespace_list in order 45dc7b236dcb modpost: use doubly linked list for dump_lists 2a322506403a modpost: traverse unresolved symbols in order a85718443348 modpost: add sym_add_unresolved() helper 5c44b0f89c82 modpost: traverse modules in order a0b68f6655f2 modpost: import include/linux/list.h ce9f4d32be4e modpost: change mod->gpl_compatible to bool type f9fe36a515ca modpost: use bool type where appropriate 46f6334d7055 modpost: move struct namespace_list to modpost.c afa24c45af49 modpost: retrieve the module dependency and CRCs in check_exports() a8f687dc3ac2 modpost: add a separate error for exported symbols without definition f97f0e32b230 modpost: remove stale comment about sym_add_exported() 0af2ad9d11c3 modpost: do not write out any file when error occurred 09eac5681c02 modpost: use snprintf() instead of sprintf() for safety ee07380110f2 kbuild: read *.mod to get objects passed to $(LD) or $(AR) 97976e5c6d55 kbuild: make *.mod not depend on *.o 0d4368c8da07 kbuild: get rid of duplication in *.mod files 55f602f00903 kbuild: split the second line of *.mod into *.usyms ea9730eb0788 kbuild: reuse real-search to simplify cmd_mod 1eacf71f885a kbuild: make multi_depend work with targets in subdirectory 19c2b5b6f769 kbuild: reuse suffix-search to refactor multi_depend 75df07a9133d kbuild: refactor cmd_modversions_S 53257fbea174 kbuild: refactor cmd_modversions_c b6e50682c261 modpost: remove annoying namespace_from_kstrtabns() 1002d8f060b0 modpost: remove redundant initializes for static variables 921fbb7ab714 modpost: move export_from_secname() call to more relevant place f49c0989e01b modpost: remove useless export_from_sec() 7a98501a77db kbuild: do not remove empty *.symtypes explicitly 500f1b31c16f kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} e04fcad29aa3 kallsyms: ignore all local labels prefixed by '.L' 9e01f7ef15d2 kbuild: drop $(size_append) from cmd_zstd 054133567480 kbuild: do not include include/config/auto.conf from shell scripts 34d14831eecb kbuild: stop using config_filename in scripts/Makefile.modsign 75155bda5498 kbuild: use more subdir- for visiting subdirectories while cleaning 1a3f00cd3be8 kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules 47704d10e997 kbuild: detect objtool update without using .SECONDEXPANSION 7a89d034ccc6 kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro 3cbbf4b9d188 kbuild: store the objtool command in *.cmd files 467f0d0aa6b4 kbuild: rename __objtool_obj and reuse it for cmd_cc_lto_link_modules
There may be a few more patches post v5.19-rc1 for Fixes. I haven't tried minimizing the 54.
Greg - is 54 too many?
Yes.
How about we just revert the original problem commit here to solve this mess? Wouldn't that be easier overall?
thanks,
greg k-h
What about a partial revert like:
diff --git a/Makefile b/Makefile index 9f5d2e87150e..aa0f7578653d 100644 --- a/Makefile +++ b/Makefile @@ -1083,7 +1083,9 @@ KBUILD_CFLAGS += $(KCFLAGS) KBUILD_LDFLAGS_MODULE += --build-id=sha1 LDFLAGS_vmlinux += --build-id=sha1
+ifneq ($(ARCH),$(filter $(ARCH),arm64)) KBUILD_LDFLAGS += -z noexecstack +endif ifeq ($(CONFIG_LD_IS_BFD),y) KBUILD_LDFLAGS += $(call ld-option,--no-warn-rwx-segments) endif
Only arm64 gcc/ld builds would need to change (with the option of adding other architectures if anyone reports same issue).
With a full revert we lose --no-warn-rwx-segments and warnings show-up with later versions of ld.
I did open a bug against 'ld' as Nick requested: https://sourceware.org/bugzilla/show_bug.cgi?id=29994
If this is this is a better way to go - I can form up a v3 patch.
--Tom
On Thu, Jan 12, 2023 at 03:20:11PM -0600, Tom Saeger wrote:
On Thu, Jan 12, 2023 at 01:00:57PM +0100, Greg Kroah-Hartman wrote:
On Mon, Jan 09, 2023 at 12:36:15PM -0600, Tom Saeger wrote:
On Thu, Dec 22, 2022 at 07:01:11AM +0100, Greg Kroah-Hartman wrote:
On Wed, Dec 21, 2022 at 02:52:10PM -0600, Tom Saeger wrote:
On Wed, Dec 21, 2022 at 05:31:51PM +0100, Greg Kroah-Hartman wrote:
On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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") > > 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")
Why can't we add this one instead of a custom change?
I quickly abandoned that route - there are too many dependencies.
How many? Why? Whenever we add a "this is not upstream" patch, 90% of the time it is incorrect and causes problems (merge issues included.) So please please please let's try to keep in sync with what is in Linus's tree.
thanks,
greg k-h
Ok - I spent some time on this.
The haystack I searched:
git rev-list --grep="masahiroy/linux-kbuild" v5.15..v5.19-rc1 | while read -r CID ; do git rev-list "${CID}^-" ; done | wc -l 182
I have 54 of those 182 applied to 5.15.85, and this works in my limited build testing (x86_64 gcc, arm64 gcc, arm64 clang).
Specifically:
cbfc9bf3223f genksyms: adjust the output format to modpost e7c9c2630e59 kbuild: stop merging *.symversions 1d788aa800c7 kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS 8a01c770955b modpost: extract symbol versions from *.cmd files a8ade6b33772 modpost: add sym_find_with_module() helper a9639fe6b516 modpost: change the license of EXPORT_SYMBOL to bool type 04804878f631 modpost: remove left-over cross_compile declaration 3388b8af9698 kbuild: record symbol versions in *.cmd files 4ff3946463a0 kbuild: generate a list of objects in vmlinux 074617e2ad6a modpost: move *.mod.c generation to write_mod_c_files() 81b78cb6e821 modpost: merge add_{intree_flag,retpoline,staging_flag} to add_header 9df4f00b53b4 modpost: split new_symbol() to symbol allocation and hash table addition 85728bcbc500 modpost: make sym_add_exported() always allocate a new symbol 82aa2b4d30af modpost: make multiple export error 6cc962f0a175 modpost: dump Module.symvers in the same order of modules.order 39db82cea373 modpost: traverse the namespace_list in order 45dc7b236dcb modpost: use doubly linked list for dump_lists 2a322506403a modpost: traverse unresolved symbols in order a85718443348 modpost: add sym_add_unresolved() helper 5c44b0f89c82 modpost: traverse modules in order a0b68f6655f2 modpost: import include/linux/list.h ce9f4d32be4e modpost: change mod->gpl_compatible to bool type f9fe36a515ca modpost: use bool type where appropriate 46f6334d7055 modpost: move struct namespace_list to modpost.c afa24c45af49 modpost: retrieve the module dependency and CRCs in check_exports() a8f687dc3ac2 modpost: add a separate error for exported symbols without definition f97f0e32b230 modpost: remove stale comment about sym_add_exported() 0af2ad9d11c3 modpost: do not write out any file when error occurred 09eac5681c02 modpost: use snprintf() instead of sprintf() for safety ee07380110f2 kbuild: read *.mod to get objects passed to $(LD) or $(AR) 97976e5c6d55 kbuild: make *.mod not depend on *.o 0d4368c8da07 kbuild: get rid of duplication in *.mod files 55f602f00903 kbuild: split the second line of *.mod into *.usyms ea9730eb0788 kbuild: reuse real-search to simplify cmd_mod 1eacf71f885a kbuild: make multi_depend work with targets in subdirectory 19c2b5b6f769 kbuild: reuse suffix-search to refactor multi_depend 75df07a9133d kbuild: refactor cmd_modversions_S 53257fbea174 kbuild: refactor cmd_modversions_c b6e50682c261 modpost: remove annoying namespace_from_kstrtabns() 1002d8f060b0 modpost: remove redundant initializes for static variables 921fbb7ab714 modpost: move export_from_secname() call to more relevant place f49c0989e01b modpost: remove useless export_from_sec() 7a98501a77db kbuild: do not remove empty *.symtypes explicitly 500f1b31c16f kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} e04fcad29aa3 kallsyms: ignore all local labels prefixed by '.L' 9e01f7ef15d2 kbuild: drop $(size_append) from cmd_zstd 054133567480 kbuild: do not include include/config/auto.conf from shell scripts 34d14831eecb kbuild: stop using config_filename in scripts/Makefile.modsign 75155bda5498 kbuild: use more subdir- for visiting subdirectories while cleaning 1a3f00cd3be8 kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules 47704d10e997 kbuild: detect objtool update without using .SECONDEXPANSION 7a89d034ccc6 kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro 3cbbf4b9d188 kbuild: store the objtool command in *.cmd files 467f0d0aa6b4 kbuild: rename __objtool_obj and reuse it for cmd_cc_lto_link_modules
There may be a few more patches post v5.19-rc1 for Fixes. I haven't tried minimizing the 54.
Greg - is 54 too many?
Yes.
How about we just revert the original problem commit here to solve this mess? Wouldn't that be easier overall?
thanks,
greg k-h
What about a partial revert like:
diff --git a/Makefile b/Makefile index 9f5d2e87150e..aa0f7578653d 100644 --- a/Makefile +++ b/Makefile @@ -1083,7 +1083,9 @@ KBUILD_CFLAGS += $(KCFLAGS) KBUILD_LDFLAGS_MODULE += --build-id=sha1 LDFLAGS_vmlinux += --build-id=sha1
+ifneq ($(ARCH),$(filter $(ARCH),arm64)) KBUILD_LDFLAGS += -z noexecstack +endif ifeq ($(CONFIG_LD_IS_BFD),y) KBUILD_LDFLAGS += $(call ld-option,--no-warn-rwx-segments) endif
Only arm64 gcc/ld builds would need to change (with the option of adding other architectures if anyone reports same issue).
With a full revert we lose --no-warn-rwx-segments and warnings show-up with later versions of ld.
I did open a bug against 'ld' as Nick requested: https://sourceware.org/bugzilla/show_bug.cgi?id=29994
If this is this is a better way to go - I can form up a v3 patch.
--Tom
nevermind
The patch discussed here fixes arm64 Build ID for 5.15, 5.10, and 5.4:
https://lore.kernel.org/all/CAMj1kXHqQoqoys83nEp=Q6oT68+-GpCuMjfnYK9pMy-X_+j...
Regards,
--Tom
On Fri, Jan 13, 2023 at 09:06:54AM -0600, Tom Saeger wrote:
On Thu, Jan 12, 2023 at 03:20:11PM -0600, Tom Saeger wrote:
On Thu, Jan 12, 2023 at 01:00:57PM +0100, Greg Kroah-Hartman wrote:
On Mon, Jan 09, 2023 at 12:36:15PM -0600, Tom Saeger wrote:
On Thu, Dec 22, 2022 at 07:01:11AM +0100, Greg Kroah-Hartman wrote:
On Wed, Dec 21, 2022 at 02:52:10PM -0600, Tom Saeger wrote:
On Wed, Dec 21, 2022 at 05:31:51PM +0100, Greg Kroah-Hartman wrote: > On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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") > > > > 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") > > Why can't we add this one instead of a custom change?
I quickly abandoned that route - there are too many dependencies.
How many? Why? Whenever we add a "this is not upstream" patch, 90% of the time it is incorrect and causes problems (merge issues included.) So please please please let's try to keep in sync with what is in Linus's tree.
thanks,
greg k-h
Ok - I spent some time on this.
The haystack I searched:
git rev-list --grep="masahiroy/linux-kbuild" v5.15..v5.19-rc1 | while read -r CID ; do git rev-list "${CID}^-" ; done | wc -l 182
I have 54 of those 182 applied to 5.15.85, and this works in my limited build testing (x86_64 gcc, arm64 gcc, arm64 clang).
Specifically:
cbfc9bf3223f genksyms: adjust the output format to modpost e7c9c2630e59 kbuild: stop merging *.symversions 1d788aa800c7 kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS 8a01c770955b modpost: extract symbol versions from *.cmd files a8ade6b33772 modpost: add sym_find_with_module() helper a9639fe6b516 modpost: change the license of EXPORT_SYMBOL to bool type 04804878f631 modpost: remove left-over cross_compile declaration 3388b8af9698 kbuild: record symbol versions in *.cmd files 4ff3946463a0 kbuild: generate a list of objects in vmlinux 074617e2ad6a modpost: move *.mod.c generation to write_mod_c_files() 81b78cb6e821 modpost: merge add_{intree_flag,retpoline,staging_flag} to add_header 9df4f00b53b4 modpost: split new_symbol() to symbol allocation and hash table addition 85728bcbc500 modpost: make sym_add_exported() always allocate a new symbol 82aa2b4d30af modpost: make multiple export error 6cc962f0a175 modpost: dump Module.symvers in the same order of modules.order 39db82cea373 modpost: traverse the namespace_list in order 45dc7b236dcb modpost: use doubly linked list for dump_lists 2a322506403a modpost: traverse unresolved symbols in order a85718443348 modpost: add sym_add_unresolved() helper 5c44b0f89c82 modpost: traverse modules in order a0b68f6655f2 modpost: import include/linux/list.h ce9f4d32be4e modpost: change mod->gpl_compatible to bool type f9fe36a515ca modpost: use bool type where appropriate 46f6334d7055 modpost: move struct namespace_list to modpost.c afa24c45af49 modpost: retrieve the module dependency and CRCs in check_exports() a8f687dc3ac2 modpost: add a separate error for exported symbols without definition f97f0e32b230 modpost: remove stale comment about sym_add_exported() 0af2ad9d11c3 modpost: do not write out any file when error occurred 09eac5681c02 modpost: use snprintf() instead of sprintf() for safety ee07380110f2 kbuild: read *.mod to get objects passed to $(LD) or $(AR) 97976e5c6d55 kbuild: make *.mod not depend on *.o 0d4368c8da07 kbuild: get rid of duplication in *.mod files 55f602f00903 kbuild: split the second line of *.mod into *.usyms ea9730eb0788 kbuild: reuse real-search to simplify cmd_mod 1eacf71f885a kbuild: make multi_depend work with targets in subdirectory 19c2b5b6f769 kbuild: reuse suffix-search to refactor multi_depend 75df07a9133d kbuild: refactor cmd_modversions_S 53257fbea174 kbuild: refactor cmd_modversions_c b6e50682c261 modpost: remove annoying namespace_from_kstrtabns() 1002d8f060b0 modpost: remove redundant initializes for static variables 921fbb7ab714 modpost: move export_from_secname() call to more relevant place f49c0989e01b modpost: remove useless export_from_sec() 7a98501a77db kbuild: do not remove empty *.symtypes explicitly 500f1b31c16f kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} e04fcad29aa3 kallsyms: ignore all local labels prefixed by '.L' 9e01f7ef15d2 kbuild: drop $(size_append) from cmd_zstd 054133567480 kbuild: do not include include/config/auto.conf from shell scripts 34d14831eecb kbuild: stop using config_filename in scripts/Makefile.modsign 75155bda5498 kbuild: use more subdir- for visiting subdirectories while cleaning 1a3f00cd3be8 kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules 47704d10e997 kbuild: detect objtool update without using .SECONDEXPANSION 7a89d034ccc6 kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro 3cbbf4b9d188 kbuild: store the objtool command in *.cmd files 467f0d0aa6b4 kbuild: rename __objtool_obj and reuse it for cmd_cc_lto_link_modules
There may be a few more patches post v5.19-rc1 for Fixes. I haven't tried minimizing the 54.
Greg - is 54 too many?
Yes.
How about we just revert the original problem commit here to solve this mess? Wouldn't that be easier overall?
thanks,
greg k-h
What about a partial revert like:
diff --git a/Makefile b/Makefile index 9f5d2e87150e..aa0f7578653d 100644 --- a/Makefile +++ b/Makefile @@ -1083,7 +1083,9 @@ KBUILD_CFLAGS += $(KCFLAGS) KBUILD_LDFLAGS_MODULE += --build-id=sha1 LDFLAGS_vmlinux += --build-id=sha1
+ifneq ($(ARCH),$(filter $(ARCH),arm64)) KBUILD_LDFLAGS += -z noexecstack +endif ifeq ($(CONFIG_LD_IS_BFD),y) KBUILD_LDFLAGS += $(call ld-option,--no-warn-rwx-segments) endif
Only arm64 gcc/ld builds would need to change (with the option of adding other architectures if anyone reports same issue).
With a full revert we lose --no-warn-rwx-segments and warnings show-up with later versions of ld.
I did open a bug against 'ld' as Nick requested: https://sourceware.org/bugzilla/show_bug.cgi?id=29994
If this is this is a better way to go - I can form up a v3 patch.
--Tom
nevermind
The patch discussed here fixes arm64 Build ID for 5.15, 5.10, and 5.4:
https://lore.kernel.org/all/CAMj1kXHqQoqoys83nEp=Q6oT68+-GpCuMjfnYK9pMy-X_+j...
Great, please let me know when this hits Linus's tree and I will be glad to queue it up.
thanks,
greg k-h
On Sat, Jan 14, 2023 at 02:53:52PM +0100, Greg Kroah-Hartman wrote:
On Fri, Jan 13, 2023 at 09:06:54AM -0600, Tom Saeger wrote:
On Thu, Jan 12, 2023 at 03:20:11PM -0600, Tom Saeger wrote:
On Thu, Jan 12, 2023 at 01:00:57PM +0100, Greg Kroah-Hartman wrote:
On Mon, Jan 09, 2023 at 12:36:15PM -0600, Tom Saeger wrote:
On Thu, Dec 22, 2022 at 07:01:11AM +0100, Greg Kroah-Hartman wrote:
On Wed, Dec 21, 2022 at 02:52:10PM -0600, Tom Saeger wrote: > On Wed, Dec 21, 2022 at 05:31:51PM +0100, Greg Kroah-Hartman wrote: > > On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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") > > > > > > 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") > > > > Why can't we add this one instead of a custom change? > > I quickly abandoned that route - there are too many dependencies.
How many? Why? Whenever we add a "this is not upstream" patch, 90% of the time it is incorrect and causes problems (merge issues included.) So please please please let's try to keep in sync with what is in Linus's tree.
thanks,
greg k-h
Ok - I spent some time on this.
The haystack I searched:
git rev-list --grep="masahiroy/linux-kbuild" v5.15..v5.19-rc1 | while read -r CID ; do git rev-list "${CID}^-" ; done | wc -l 182
I have 54 of those 182 applied to 5.15.85, and this works in my limited build testing (x86_64 gcc, arm64 gcc, arm64 clang).
Specifically:
cbfc9bf3223f genksyms: adjust the output format to modpost e7c9c2630e59 kbuild: stop merging *.symversions 1d788aa800c7 kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS 8a01c770955b modpost: extract symbol versions from *.cmd files a8ade6b33772 modpost: add sym_find_with_module() helper a9639fe6b516 modpost: change the license of EXPORT_SYMBOL to bool type 04804878f631 modpost: remove left-over cross_compile declaration 3388b8af9698 kbuild: record symbol versions in *.cmd files 4ff3946463a0 kbuild: generate a list of objects in vmlinux 074617e2ad6a modpost: move *.mod.c generation to write_mod_c_files() 81b78cb6e821 modpost: merge add_{intree_flag,retpoline,staging_flag} to add_header 9df4f00b53b4 modpost: split new_symbol() to symbol allocation and hash table addition 85728bcbc500 modpost: make sym_add_exported() always allocate a new symbol 82aa2b4d30af modpost: make multiple export error 6cc962f0a175 modpost: dump Module.symvers in the same order of modules.order 39db82cea373 modpost: traverse the namespace_list in order 45dc7b236dcb modpost: use doubly linked list for dump_lists 2a322506403a modpost: traverse unresolved symbols in order a85718443348 modpost: add sym_add_unresolved() helper 5c44b0f89c82 modpost: traverse modules in order a0b68f6655f2 modpost: import include/linux/list.h ce9f4d32be4e modpost: change mod->gpl_compatible to bool type f9fe36a515ca modpost: use bool type where appropriate 46f6334d7055 modpost: move struct namespace_list to modpost.c afa24c45af49 modpost: retrieve the module dependency and CRCs in check_exports() a8f687dc3ac2 modpost: add a separate error for exported symbols without definition f97f0e32b230 modpost: remove stale comment about sym_add_exported() 0af2ad9d11c3 modpost: do not write out any file when error occurred 09eac5681c02 modpost: use snprintf() instead of sprintf() for safety ee07380110f2 kbuild: read *.mod to get objects passed to $(LD) or $(AR) 97976e5c6d55 kbuild: make *.mod not depend on *.o 0d4368c8da07 kbuild: get rid of duplication in *.mod files 55f602f00903 kbuild: split the second line of *.mod into *.usyms ea9730eb0788 kbuild: reuse real-search to simplify cmd_mod 1eacf71f885a kbuild: make multi_depend work with targets in subdirectory 19c2b5b6f769 kbuild: reuse suffix-search to refactor multi_depend 75df07a9133d kbuild: refactor cmd_modversions_S 53257fbea174 kbuild: refactor cmd_modversions_c b6e50682c261 modpost: remove annoying namespace_from_kstrtabns() 1002d8f060b0 modpost: remove redundant initializes for static variables 921fbb7ab714 modpost: move export_from_secname() call to more relevant place f49c0989e01b modpost: remove useless export_from_sec() 7a98501a77db kbuild: do not remove empty *.symtypes explicitly 500f1b31c16f kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} e04fcad29aa3 kallsyms: ignore all local labels prefixed by '.L' 9e01f7ef15d2 kbuild: drop $(size_append) from cmd_zstd 054133567480 kbuild: do not include include/config/auto.conf from shell scripts 34d14831eecb kbuild: stop using config_filename in scripts/Makefile.modsign 75155bda5498 kbuild: use more subdir- for visiting subdirectories while cleaning 1a3f00cd3be8 kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules 47704d10e997 kbuild: detect objtool update without using .SECONDEXPANSION 7a89d034ccc6 kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro 3cbbf4b9d188 kbuild: store the objtool command in *.cmd files 467f0d0aa6b4 kbuild: rename __objtool_obj and reuse it for cmd_cc_lto_link_modules
There may be a few more patches post v5.19-rc1 for Fixes. I haven't tried minimizing the 54.
Greg - is 54 too many?
Yes.
How about we just revert the original problem commit here to solve this mess? Wouldn't that be easier overall?
thanks,
greg k-h
What about a partial revert like:
diff --git a/Makefile b/Makefile index 9f5d2e87150e..aa0f7578653d 100644 --- a/Makefile +++ b/Makefile @@ -1083,7 +1083,9 @@ KBUILD_CFLAGS += $(KCFLAGS) KBUILD_LDFLAGS_MODULE += --build-id=sha1 LDFLAGS_vmlinux += --build-id=sha1
+ifneq ($(ARCH),$(filter $(ARCH),arm64)) KBUILD_LDFLAGS += -z noexecstack +endif ifeq ($(CONFIG_LD_IS_BFD),y) KBUILD_LDFLAGS += $(call ld-option,--no-warn-rwx-segments) endif
Only arm64 gcc/ld builds would need to change (with the option of adding other architectures if anyone reports same issue).
With a full revert we lose --no-warn-rwx-segments and warnings show-up with later versions of ld.
I did open a bug against 'ld' as Nick requested: https://sourceware.org/bugzilla/show_bug.cgi?id=29994
If this is this is a better way to go - I can form up a v3 patch.
--Tom
nevermind
The patch discussed here fixes arm64 Build ID for 5.15, 5.10, and 5.4:
https://lore.kernel.org/all/CAMj1kXHqQoqoys83nEp=Q6oT68+-GpCuMjfnYK9pMy-X_+j...
Great, please let me know when this hits Linus's tree and I will be glad to queue it up.
thanks,
greg k-h
Hi Greg,
Masahiroy's commit is already in Linus's tree.
❯ git log -n1 --format=oneline 99cb0d917ffa 99cb0d917ffa1ab628bb67364ca9b162c07699b1 arch: fix broken BuildID for arm64 and riscv
❯ git tag --contains=99cb0d917ffa v6.2-rc2 v6.2-rc3 v6.2-rc4
Needed to fix Build ID in 5.15, 5.10, and 5.4
Build results on arm64: PASS v4.19.269 c652c812211c ("Linux 4.19.269") Build ID: 3b638c635fb3f3241b3e7ad6a147cf69d931b5b7 PASS v4.19.269 00527d2a4998 ("arch: fix broken BuildID for arm64 and riscv") Build ID: 919b5ca1964776926bc6c8addc5b8af4fb15367b FAIL v5.4.228 851c2b5fb793 ("Linux 5.4.228") PASS v5.4.228 39bb8287bc08 ("arch: fix broken BuildID for arm64 and riscv") Build ID: 483ac60fe71545045e625e681f3d4ebae5d15cd1 FAIL v5.10.163 19ff2d645f7a ("Linux 5.10.163") PASS v5.10.163 6136c3a732cf ("arch: fix broken BuildID for arm64 and riscv") Build ID: 4c0926311f96a031c0581d8136d09ca4f7ca77b6 FAIL v5.15.88 90bb4f8f399f ("Linux 5.15.88") PASS v5.15.88 6cb364966c77 ("arch: fix broken BuildID for arm64 and riscv") Build ID: 623dab2f6bd78271e315493c232abf042af88036 PASS v6.1.6 38f3ee12661f ("Linux 6.1.6") Build ID: 8b9e3e330b093ab6037a5dcffcaefca84a878d44 PASS v6.1.6 db1031af82be ("arch: fix broken BuildID for arm64 and riscv") Build ID: 2d848c31fcc31414513fa33ff2990fe6c9afa88c
Regards,
--Tom
On Tue, Jan 17, 2023 at 05:50:06PM -0600, Tom Saeger wrote:
On Sat, Jan 14, 2023 at 02:53:52PM +0100, Greg Kroah-Hartman wrote: Masahiroy's commit is already in Linus's tree.
❯ git log -n1 --format=oneline 99cb0d917ffa 99cb0d917ffa1ab628bb67364ca9b162c07699b1 arch: fix broken BuildID for arm64 and riscv
❯ git tag --contains=99cb0d917ffa v6.2-rc2 v6.2-rc3 v6.2-rc4
Using 'git tag' doesn't always show the best info, better is the following: $ git describe --contains 99cb0d917ffa1ab628bb67364ca9b162c07699b1 v6.2-rc2~5^2~6
Anyway, I'll look at this after the next round gets released, thanks!
greg k-h
On Tue, Jan 17, 2023 at 05:50:06PM -0600, Tom Saeger wrote:
On Sat, Jan 14, 2023 at 02:53:52PM +0100, Greg Kroah-Hartman wrote:
On Fri, Jan 13, 2023 at 09:06:54AM -0600, Tom Saeger wrote:
On Thu, Jan 12, 2023 at 03:20:11PM -0600, Tom Saeger wrote:
On Thu, Jan 12, 2023 at 01:00:57PM +0100, Greg Kroah-Hartman wrote:
On Mon, Jan 09, 2023 at 12:36:15PM -0600, Tom Saeger wrote:
On Thu, Dec 22, 2022 at 07:01:11AM +0100, Greg Kroah-Hartman wrote: > On Wed, Dec 21, 2022 at 02:52:10PM -0600, Tom Saeger wrote: > > On Wed, Dec 21, 2022 at 05:31:51PM +0100, Greg Kroah-Hartman wrote: > > > On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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") > > > > > > > > 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") > > > > > > Why can't we add this one instead of a custom change? > > > > I quickly abandoned that route - there are too many dependencies. > > How many? Why? Whenever we add a "this is not upstream" patch, 90% of > the time it is incorrect and causes problems (merge issues included.) > So please please please let's try to keep in sync with what is in > Linus's tree. > > thanks, > > greg k-h
Ok - I spent some time on this.
The haystack I searched:
git rev-list --grep="masahiroy/linux-kbuild" v5.15..v5.19-rc1 | while read -r CID ; do git rev-list "${CID}^-" ; done | wc -l 182
I have 54 of those 182 applied to 5.15.85, and this works in my limited build testing (x86_64 gcc, arm64 gcc, arm64 clang).
Specifically:
cbfc9bf3223f genksyms: adjust the output format to modpost e7c9c2630e59 kbuild: stop merging *.symversions 1d788aa800c7 kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS 8a01c770955b modpost: extract symbol versions from *.cmd files a8ade6b33772 modpost: add sym_find_with_module() helper a9639fe6b516 modpost: change the license of EXPORT_SYMBOL to bool type 04804878f631 modpost: remove left-over cross_compile declaration 3388b8af9698 kbuild: record symbol versions in *.cmd files 4ff3946463a0 kbuild: generate a list of objects in vmlinux 074617e2ad6a modpost: move *.mod.c generation to write_mod_c_files() 81b78cb6e821 modpost: merge add_{intree_flag,retpoline,staging_flag} to add_header 9df4f00b53b4 modpost: split new_symbol() to symbol allocation and hash table addition 85728bcbc500 modpost: make sym_add_exported() always allocate a new symbol 82aa2b4d30af modpost: make multiple export error 6cc962f0a175 modpost: dump Module.symvers in the same order of modules.order 39db82cea373 modpost: traverse the namespace_list in order 45dc7b236dcb modpost: use doubly linked list for dump_lists 2a322506403a modpost: traverse unresolved symbols in order a85718443348 modpost: add sym_add_unresolved() helper 5c44b0f89c82 modpost: traverse modules in order a0b68f6655f2 modpost: import include/linux/list.h ce9f4d32be4e modpost: change mod->gpl_compatible to bool type f9fe36a515ca modpost: use bool type where appropriate 46f6334d7055 modpost: move struct namespace_list to modpost.c afa24c45af49 modpost: retrieve the module dependency and CRCs in check_exports() a8f687dc3ac2 modpost: add a separate error for exported symbols without definition f97f0e32b230 modpost: remove stale comment about sym_add_exported() 0af2ad9d11c3 modpost: do not write out any file when error occurred 09eac5681c02 modpost: use snprintf() instead of sprintf() for safety ee07380110f2 kbuild: read *.mod to get objects passed to $(LD) or $(AR) 97976e5c6d55 kbuild: make *.mod not depend on *.o 0d4368c8da07 kbuild: get rid of duplication in *.mod files 55f602f00903 kbuild: split the second line of *.mod into *.usyms ea9730eb0788 kbuild: reuse real-search to simplify cmd_mod 1eacf71f885a kbuild: make multi_depend work with targets in subdirectory 19c2b5b6f769 kbuild: reuse suffix-search to refactor multi_depend 75df07a9133d kbuild: refactor cmd_modversions_S 53257fbea174 kbuild: refactor cmd_modversions_c b6e50682c261 modpost: remove annoying namespace_from_kstrtabns() 1002d8f060b0 modpost: remove redundant initializes for static variables 921fbb7ab714 modpost: move export_from_secname() call to more relevant place f49c0989e01b modpost: remove useless export_from_sec() 7a98501a77db kbuild: do not remove empty *.symtypes explicitly 500f1b31c16f kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} e04fcad29aa3 kallsyms: ignore all local labels prefixed by '.L' 9e01f7ef15d2 kbuild: drop $(size_append) from cmd_zstd 054133567480 kbuild: do not include include/config/auto.conf from shell scripts 34d14831eecb kbuild: stop using config_filename in scripts/Makefile.modsign 75155bda5498 kbuild: use more subdir- for visiting subdirectories while cleaning 1a3f00cd3be8 kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules 47704d10e997 kbuild: detect objtool update without using .SECONDEXPANSION 7a89d034ccc6 kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro 3cbbf4b9d188 kbuild: store the objtool command in *.cmd files 467f0d0aa6b4 kbuild: rename __objtool_obj and reuse it for cmd_cc_lto_link_modules
There may be a few more patches post v5.19-rc1 for Fixes. I haven't tried minimizing the 54.
Greg - is 54 too many?
Yes.
How about we just revert the original problem commit here to solve this mess? Wouldn't that be easier overall?
thanks,
greg k-h
What about a partial revert like:
diff --git a/Makefile b/Makefile index 9f5d2e87150e..aa0f7578653d 100644 --- a/Makefile +++ b/Makefile @@ -1083,7 +1083,9 @@ KBUILD_CFLAGS += $(KCFLAGS) KBUILD_LDFLAGS_MODULE += --build-id=sha1 LDFLAGS_vmlinux += --build-id=sha1
+ifneq ($(ARCH),$(filter $(ARCH),arm64)) KBUILD_LDFLAGS += -z noexecstack +endif ifeq ($(CONFIG_LD_IS_BFD),y) KBUILD_LDFLAGS += $(call ld-option,--no-warn-rwx-segments) endif
Only arm64 gcc/ld builds would need to change (with the option of adding other architectures if anyone reports same issue).
With a full revert we lose --no-warn-rwx-segments and warnings show-up with later versions of ld.
I did open a bug against 'ld' as Nick requested: https://sourceware.org/bugzilla/show_bug.cgi?id=29994
If this is this is a better way to go - I can form up a v3 patch.
--Tom
nevermind
The patch discussed here fixes arm64 Build ID for 5.15, 5.10, and 5.4:
https://lore.kernel.org/all/CAMj1kXHqQoqoys83nEp=Q6oT68+-GpCuMjfnYK9pMy-X_+j...
Great, please let me know when this hits Linus's tree and I will be glad to queue it up.
thanks,
greg k-h
Hi Greg,
Masahiroy's commit is already in Linus's tree.
❯ git log -n1 --format=oneline 99cb0d917ffa 99cb0d917ffa1ab628bb67364ca9b162c07699b1 arch: fix broken BuildID for arm64 and riscv
❯ git tag --contains=99cb0d917ffa v6.2-rc2 v6.2-rc3 v6.2-rc4
Needed to fix Build ID in 5.15, 5.10, and 5.4
Build results on arm64: PASS v4.19.269 c652c812211c ("Linux 4.19.269") Build ID: 3b638c635fb3f3241b3e7ad6a147cf69d931b5b7 PASS v4.19.269 00527d2a4998 ("arch: fix broken BuildID for arm64 and riscv") Build ID: 919b5ca1964776926bc6c8addc5b8af4fb15367b FAIL v5.4.228 851c2b5fb793 ("Linux 5.4.228") PASS v5.4.228 39bb8287bc08 ("arch: fix broken BuildID for arm64 and riscv") Build ID: 483ac60fe71545045e625e681f3d4ebae5d15cd1 FAIL v5.10.163 19ff2d645f7a ("Linux 5.10.163") PASS v5.10.163 6136c3a732cf ("arch: fix broken BuildID for arm64 and riscv") Build ID: 4c0926311f96a031c0581d8136d09ca4f7ca77b6 FAIL v5.15.88 90bb4f8f399f ("Linux 5.15.88") PASS v5.15.88 6cb364966c77 ("arch: fix broken BuildID for arm64 and riscv") Build ID: 623dab2f6bd78271e315493c232abf042af88036 PASS v6.1.6 38f3ee12661f ("Linux 6.1.6") Build ID: 8b9e3e330b093ab6037a5dcffcaefca84a878d44 PASS v6.1.6 db1031af82be ("arch: fix broken BuildID for arm64 and riscv") Build ID: 2d848c31fcc31414513fa33ff2990fe6c9afa88c
Now queued up everywhere, thanks!
greg k-h
On Sun, Jan 22, 2023 at 03:21:43PM +0100, Greg Kroah-Hartman wrote:
On Tue, Jan 17, 2023 at 05:50:06PM -0600, Tom Saeger wrote:
On Sat, Jan 14, 2023 at 02:53:52PM +0100, Greg Kroah-Hartman wrote:
On Fri, Jan 13, 2023 at 09:06:54AM -0600, Tom Saeger wrote:
On Thu, Jan 12, 2023 at 03:20:11PM -0600, Tom Saeger wrote:
On Thu, Jan 12, 2023 at 01:00:57PM +0100, Greg Kroah-Hartman wrote:
On Mon, Jan 09, 2023 at 12:36:15PM -0600, Tom Saeger wrote: > On Thu, Dec 22, 2022 at 07:01:11AM +0100, Greg Kroah-Hartman wrote: > > On Wed, Dec 21, 2022 at 02:52:10PM -0600, Tom Saeger wrote: > > > On Wed, Dec 21, 2022 at 05:31:51PM +0100, Greg Kroah-Hartman wrote: > > > > On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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") > > > > > > > > > > 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") > > > > > > > > Why can't we add this one instead of a custom change? > > > > > > I quickly abandoned that route - there are too many dependencies. > > > > How many? Why? Whenever we add a "this is not upstream" patch, 90% of > > the time it is incorrect and causes problems (merge issues included.) > > So please please please let's try to keep in sync with what is in > > Linus's tree. > > > > thanks, > > > > greg k-h > > Ok - I spent some time on this. > > The haystack I searched: > > git rev-list --grep="masahiroy/linux-kbuild" v5.15..v5.19-rc1 | while read -r CID ; do git rev-list "${CID}^-" ; done | wc -l > 182 > > I have 54 of those 182 applied to 5.15.85, and this works in my > limited build testing (x86_64 gcc, arm64 gcc, arm64 clang). > > Specifically: > > > cbfc9bf3223f genksyms: adjust the output format to modpost > e7c9c2630e59 kbuild: stop merging *.symversions > 1d788aa800c7 kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS > 8a01c770955b modpost: extract symbol versions from *.cmd files > a8ade6b33772 modpost: add sym_find_with_module() helper > a9639fe6b516 modpost: change the license of EXPORT_SYMBOL to bool type > 04804878f631 modpost: remove left-over cross_compile declaration > 3388b8af9698 kbuild: record symbol versions in *.cmd files > 4ff3946463a0 kbuild: generate a list of objects in vmlinux > 074617e2ad6a modpost: move *.mod.c generation to write_mod_c_files() > 81b78cb6e821 modpost: merge add_{intree_flag,retpoline,staging_flag} to add_header > 9df4f00b53b4 modpost: split new_symbol() to symbol allocation and hash table addition > 85728bcbc500 modpost: make sym_add_exported() always allocate a new symbol > 82aa2b4d30af modpost: make multiple export error > 6cc962f0a175 modpost: dump Module.symvers in the same order of modules.order > 39db82cea373 modpost: traverse the namespace_list in order > 45dc7b236dcb modpost: use doubly linked list for dump_lists > 2a322506403a modpost: traverse unresolved symbols in order > a85718443348 modpost: add sym_add_unresolved() helper > 5c44b0f89c82 modpost: traverse modules in order > a0b68f6655f2 modpost: import include/linux/list.h > ce9f4d32be4e modpost: change mod->gpl_compatible to bool type > f9fe36a515ca modpost: use bool type where appropriate > 46f6334d7055 modpost: move struct namespace_list to modpost.c > afa24c45af49 modpost: retrieve the module dependency and CRCs in check_exports() > a8f687dc3ac2 modpost: add a separate error for exported symbols without definition > f97f0e32b230 modpost: remove stale comment about sym_add_exported() > 0af2ad9d11c3 modpost: do not write out any file when error occurred > 09eac5681c02 modpost: use snprintf() instead of sprintf() for safety > ee07380110f2 kbuild: read *.mod to get objects passed to $(LD) or $(AR) > 97976e5c6d55 kbuild: make *.mod not depend on *.o > 0d4368c8da07 kbuild: get rid of duplication in *.mod files > 55f602f00903 kbuild: split the second line of *.mod into *.usyms > ea9730eb0788 kbuild: reuse real-search to simplify cmd_mod > 1eacf71f885a kbuild: make multi_depend work with targets in subdirectory > 19c2b5b6f769 kbuild: reuse suffix-search to refactor multi_depend > 75df07a9133d kbuild: refactor cmd_modversions_S > 53257fbea174 kbuild: refactor cmd_modversions_c > b6e50682c261 modpost: remove annoying namespace_from_kstrtabns() > 1002d8f060b0 modpost: remove redundant initializes for static variables > 921fbb7ab714 modpost: move export_from_secname() call to more relevant place > f49c0989e01b modpost: remove useless export_from_sec() > 7a98501a77db kbuild: do not remove empty *.symtypes explicitly > 500f1b31c16f kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} > e04fcad29aa3 kallsyms: ignore all local labels prefixed by '.L' > 9e01f7ef15d2 kbuild: drop $(size_append) from cmd_zstd > 054133567480 kbuild: do not include include/config/auto.conf from shell scripts > 34d14831eecb kbuild: stop using config_filename in scripts/Makefile.modsign > 75155bda5498 kbuild: use more subdir- for visiting subdirectories while cleaning > 1a3f00cd3be8 kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules > 47704d10e997 kbuild: detect objtool update without using .SECONDEXPANSION > 7a89d034ccc6 kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro > 3cbbf4b9d188 kbuild: store the objtool command in *.cmd files > 467f0d0aa6b4 kbuild: rename __objtool_obj and reuse it for cmd_cc_lto_link_modules > > There may be a few more patches post v5.19-rc1 for Fixes. > I haven't tried minimizing the 54. > > Greg - is 54 too many?
Yes.
How about we just revert the original problem commit here to solve this mess? Wouldn't that be easier overall?
thanks,
greg k-h
What about a partial revert like:
diff --git a/Makefile b/Makefile index 9f5d2e87150e..aa0f7578653d 100644 --- a/Makefile +++ b/Makefile @@ -1083,7 +1083,9 @@ KBUILD_CFLAGS += $(KCFLAGS) KBUILD_LDFLAGS_MODULE += --build-id=sha1 LDFLAGS_vmlinux += --build-id=sha1
+ifneq ($(ARCH),$(filter $(ARCH),arm64)) KBUILD_LDFLAGS += -z noexecstack +endif ifeq ($(CONFIG_LD_IS_BFD),y) KBUILD_LDFLAGS += $(call ld-option,--no-warn-rwx-segments) endif
Only arm64 gcc/ld builds would need to change (with the option of adding other architectures if anyone reports same issue).
With a full revert we lose --no-warn-rwx-segments and warnings show-up with later versions of ld.
I did open a bug against 'ld' as Nick requested: https://sourceware.org/bugzilla/show_bug.cgi?id=29994
If this is this is a better way to go - I can form up a v3 patch.
--Tom
nevermind
The patch discussed here fixes arm64 Build ID for 5.15, 5.10, and 5.4:
https://lore.kernel.org/all/CAMj1kXHqQoqoys83nEp=Q6oT68+-GpCuMjfnYK9pMy-X_+j...
Great, please let me know when this hits Linus's tree and I will be glad to queue it up.
thanks,
greg k-h
Hi Greg,
Masahiroy's commit is already in Linus's tree.
❯ git log -n1 --format=oneline 99cb0d917ffa 99cb0d917ffa1ab628bb67364ca9b162c07699b1 arch: fix broken BuildID for arm64 and riscv
❯ git tag --contains=99cb0d917ffa v6.2-rc2 v6.2-rc3 v6.2-rc4
Needed to fix Build ID in 5.15, 5.10, and 5.4
Build results on arm64: PASS v4.19.269 c652c812211c ("Linux 4.19.269") Build ID: 3b638c635fb3f3241b3e7ad6a147cf69d931b5b7 PASS v4.19.269 00527d2a4998 ("arch: fix broken BuildID for arm64 and riscv") Build ID: 919b5ca1964776926bc6c8addc5b8af4fb15367b FAIL v5.4.228 851c2b5fb793 ("Linux 5.4.228") PASS v5.4.228 39bb8287bc08 ("arch: fix broken BuildID for arm64 and riscv") Build ID: 483ac60fe71545045e625e681f3d4ebae5d15cd1 FAIL v5.10.163 19ff2d645f7a ("Linux 5.10.163") PASS v5.10.163 6136c3a732cf ("arch: fix broken BuildID for arm64 and riscv") Build ID: 4c0926311f96a031c0581d8136d09ca4f7ca77b6 FAIL v5.15.88 90bb4f8f399f ("Linux 5.15.88") PASS v5.15.88 6cb364966c77 ("arch: fix broken BuildID for arm64 and riscv") Build ID: 623dab2f6bd78271e315493c232abf042af88036 PASS v6.1.6 38f3ee12661f ("Linux 6.1.6") Build ID: 8b9e3e330b093ab6037a5dcffcaefca84a878d44 PASS v6.1.6 db1031af82be ("arch: fix broken BuildID for arm64 and riscv") Build ID: 2d848c31fcc31414513fa33ff2990fe6c9afa88c
Now queued up everywhere, thanks!
Ick, there was lots of fix-up patches for this commit, please always be aware of that when recommending a patch be backported...
thanks,
greg k-h
On Sun, Jan 22, 2023 at 03:40:02PM +0100, Greg Kroah-Hartman wrote:
On Sun, Jan 22, 2023 at 03:21:43PM +0100, Greg Kroah-Hartman wrote:
On Tue, Jan 17, 2023 at 05:50:06PM -0600, Tom Saeger wrote:
On Sat, Jan 14, 2023 at 02:53:52PM +0100, Greg Kroah-Hartman wrote:
On Fri, Jan 13, 2023 at 09:06:54AM -0600, Tom Saeger wrote:
On Thu, Jan 12, 2023 at 03:20:11PM -0600, Tom Saeger wrote:
On Thu, Jan 12, 2023 at 01:00:57PM +0100, Greg Kroah-Hartman wrote: > On Mon, Jan 09, 2023 at 12:36:15PM -0600, Tom Saeger wrote: > > On Thu, Dec 22, 2022 at 07:01:11AM +0100, Greg Kroah-Hartman wrote: > > > On Wed, Dec 21, 2022 at 02:52:10PM -0600, Tom Saeger wrote: > > > > On Wed, Dec 21, 2022 at 05:31:51PM +0100, Greg Kroah-Hartman wrote: > > > > > On Thu, Dec 15, 2022 at 04:18:18PM -0700, Tom Saeger 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") > > > > > > > > > > > > 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") > > > > > > > > > > Why can't we add this one instead of a custom change? > > > > > > > > I quickly abandoned that route - there are too many dependencies. > > > > > > How many? Why? Whenever we add a "this is not upstream" patch, 90% of > > > the time it is incorrect and causes problems (merge issues included.) > > > So please please please let's try to keep in sync with what is in > > > Linus's tree. > > > > > > thanks, > > > > > > greg k-h > > > > Ok - I spent some time on this. > > > > The haystack I searched: > > > > git rev-list --grep="masahiroy/linux-kbuild" v5.15..v5.19-rc1 | while read -r CID ; do git rev-list "${CID}^-" ; done | wc -l > > 182 > > > > I have 54 of those 182 applied to 5.15.85, and this works in my > > limited build testing (x86_64 gcc, arm64 gcc, arm64 clang). > > > > Specifically: > > > > > > cbfc9bf3223f genksyms: adjust the output format to modpost > > e7c9c2630e59 kbuild: stop merging *.symversions > > 1d788aa800c7 kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS > > 8a01c770955b modpost: extract symbol versions from *.cmd files > > a8ade6b33772 modpost: add sym_find_with_module() helper > > a9639fe6b516 modpost: change the license of EXPORT_SYMBOL to bool type > > 04804878f631 modpost: remove left-over cross_compile declaration > > 3388b8af9698 kbuild: record symbol versions in *.cmd files > > 4ff3946463a0 kbuild: generate a list of objects in vmlinux > > 074617e2ad6a modpost: move *.mod.c generation to write_mod_c_files() > > 81b78cb6e821 modpost: merge add_{intree_flag,retpoline,staging_flag} to add_header > > 9df4f00b53b4 modpost: split new_symbol() to symbol allocation and hash table addition > > 85728bcbc500 modpost: make sym_add_exported() always allocate a new symbol > > 82aa2b4d30af modpost: make multiple export error > > 6cc962f0a175 modpost: dump Module.symvers in the same order of modules.order > > 39db82cea373 modpost: traverse the namespace_list in order > > 45dc7b236dcb modpost: use doubly linked list for dump_lists > > 2a322506403a modpost: traverse unresolved symbols in order > > a85718443348 modpost: add sym_add_unresolved() helper > > 5c44b0f89c82 modpost: traverse modules in order > > a0b68f6655f2 modpost: import include/linux/list.h > > ce9f4d32be4e modpost: change mod->gpl_compatible to bool type > > f9fe36a515ca modpost: use bool type where appropriate > > 46f6334d7055 modpost: move struct namespace_list to modpost.c > > afa24c45af49 modpost: retrieve the module dependency and CRCs in check_exports() > > a8f687dc3ac2 modpost: add a separate error for exported symbols without definition > > f97f0e32b230 modpost: remove stale comment about sym_add_exported() > > 0af2ad9d11c3 modpost: do not write out any file when error occurred > > 09eac5681c02 modpost: use snprintf() instead of sprintf() for safety > > ee07380110f2 kbuild: read *.mod to get objects passed to $(LD) or $(AR) > > 97976e5c6d55 kbuild: make *.mod not depend on *.o > > 0d4368c8da07 kbuild: get rid of duplication in *.mod files > > 55f602f00903 kbuild: split the second line of *.mod into *.usyms > > ea9730eb0788 kbuild: reuse real-search to simplify cmd_mod > > 1eacf71f885a kbuild: make multi_depend work with targets in subdirectory > > 19c2b5b6f769 kbuild: reuse suffix-search to refactor multi_depend > > 75df07a9133d kbuild: refactor cmd_modversions_S > > 53257fbea174 kbuild: refactor cmd_modversions_c > > b6e50682c261 modpost: remove annoying namespace_from_kstrtabns() > > 1002d8f060b0 modpost: remove redundant initializes for static variables > > 921fbb7ab714 modpost: move export_from_secname() call to more relevant place > > f49c0989e01b modpost: remove useless export_from_sec() > > 7a98501a77db kbuild: do not remove empty *.symtypes explicitly > > 500f1b31c16f kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} > > e04fcad29aa3 kallsyms: ignore all local labels prefixed by '.L' > > 9e01f7ef15d2 kbuild: drop $(size_append) from cmd_zstd > > 054133567480 kbuild: do not include include/config/auto.conf from shell scripts > > 34d14831eecb kbuild: stop using config_filename in scripts/Makefile.modsign > > 75155bda5498 kbuild: use more subdir- for visiting subdirectories while cleaning > > 1a3f00cd3be8 kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules > > 47704d10e997 kbuild: detect objtool update without using .SECONDEXPANSION > > 7a89d034ccc6 kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro > > 3cbbf4b9d188 kbuild: store the objtool command in *.cmd files > > 467f0d0aa6b4 kbuild: rename __objtool_obj and reuse it for cmd_cc_lto_link_modules > > > > There may be a few more patches post v5.19-rc1 for Fixes. > > I haven't tried minimizing the 54. > > > > Greg - is 54 too many? > > Yes. > > How about we just revert the original problem commit here to solve this > mess? Wouldn't that be easier overall? > > thanks, > > greg k-h
What about a partial revert like:
diff --git a/Makefile b/Makefile index 9f5d2e87150e..aa0f7578653d 100644 --- a/Makefile +++ b/Makefile @@ -1083,7 +1083,9 @@ KBUILD_CFLAGS += $(KCFLAGS) KBUILD_LDFLAGS_MODULE += --build-id=sha1 LDFLAGS_vmlinux += --build-id=sha1
+ifneq ($(ARCH),$(filter $(ARCH),arm64)) KBUILD_LDFLAGS += -z noexecstack +endif ifeq ($(CONFIG_LD_IS_BFD),y) KBUILD_LDFLAGS += $(call ld-option,--no-warn-rwx-segments) endif
Only arm64 gcc/ld builds would need to change (with the option of adding other architectures if anyone reports same issue).
With a full revert we lose --no-warn-rwx-segments and warnings show-up with later versions of ld.
I did open a bug against 'ld' as Nick requested: https://sourceware.org/bugzilla/show_bug.cgi?id=29994
If this is this is a better way to go - I can form up a v3 patch.
--Tom
nevermind
The patch discussed here fixes arm64 Build ID for 5.15, 5.10, and 5.4:
https://lore.kernel.org/all/CAMj1kXHqQoqoys83nEp=Q6oT68+-GpCuMjfnYK9pMy-X_+j...
Great, please let me know when this hits Linus's tree and I will be glad to queue it up.
thanks,
greg k-h
Hi Greg,
Masahiroy's commit is already in Linus's tree.
❯ git log -n1 --format=oneline 99cb0d917ffa 99cb0d917ffa1ab628bb67364ca9b162c07699b1 arch: fix broken BuildID for arm64 and riscv
❯ git tag --contains=99cb0d917ffa v6.2-rc2 v6.2-rc3 v6.2-rc4
Needed to fix Build ID in 5.15, 5.10, and 5.4
Build results on arm64: PASS v4.19.269 c652c812211c ("Linux 4.19.269") Build ID: 3b638c635fb3f3241b3e7ad6a147cf69d931b5b7 PASS v4.19.269 00527d2a4998 ("arch: fix broken BuildID for arm64 and riscv") Build ID: 919b5ca1964776926bc6c8addc5b8af4fb15367b FAIL v5.4.228 851c2b5fb793 ("Linux 5.4.228") PASS v5.4.228 39bb8287bc08 ("arch: fix broken BuildID for arm64 and riscv") Build ID: 483ac60fe71545045e625e681f3d4ebae5d15cd1 FAIL v5.10.163 19ff2d645f7a ("Linux 5.10.163") PASS v5.10.163 6136c3a732cf ("arch: fix broken BuildID for arm64 and riscv") Build ID: 4c0926311f96a031c0581d8136d09ca4f7ca77b6 FAIL v5.15.88 90bb4f8f399f ("Linux 5.15.88") PASS v5.15.88 6cb364966c77 ("arch: fix broken BuildID for arm64 and riscv") Build ID: 623dab2f6bd78271e315493c232abf042af88036 PASS v6.1.6 38f3ee12661f ("Linux 6.1.6") Build ID: 8b9e3e330b093ab6037a5dcffcaefca84a878d44 PASS v6.1.6 db1031af82be ("arch: fix broken BuildID for arm64 and riscv") Build ID: 2d848c31fcc31414513fa33ff2990fe6c9afa88c
Now queued up everywhere, thanks!
Ick, there was lots of fix-up patches for this commit, please always be aware of that when recommending a patch be backported...
And it broke the builds on all backports :(
I'm going to drop this now, and the fixup patches, from all branches. Please resubmit a set of _working_ commits for all branches that you care about, and I will be glad to reconsider them then, as obviously this was not tested very well.
thanks,
greg k-h
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@oracle.com 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-...
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@vger.kernel.org # 5.15, 5.10, 5.4 Cc: linux-kbuild@vger.kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Masahiro Yamada masahiroy@kernel.org Cc: Nicholas Piggin npiggin@gmail.com Cc: Michal Marek michal.lkml@markovi.net Cc: Nathan Chancellor nathan@kernel.org Signed-off-by: Tom Saeger tom.saeger@oracle.com
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@oracle.com/
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
On Wed, Dec 21, 2022 at 11:56:33AM -0800, Nick Desaulniers wrote:
Tom, thanks for pursuing this. Sorry I'm falling behind in reviews (going offline soon for the holidays).
Same :)
Some additional questions:
On Thu, Dec 15, 2022 at 3:18 PM Tom Saeger tom.saeger@oracle.com 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?
I've only encountered this with arm64 and specifically NOT with x86. I suspect other architectures may encounter this, but I haven't tried.
v1 cover has a simple example if someone has capability/time to adapt to another architecture.
- enable CONFIG_MODVERSIONS - build - readelf -n vmlinux
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
I've found: https://sourceware.org/bugzilla/show_bug.cgi?id=16744 Comment 1: https://sourceware.org/bugzilla/show_bug.cgi?id=16744#c1
"the semantics of a .note.GNU-stack presence is target-dependent."
corresponding to this commit: https://sourceware.org/git/?p=binutils-gdb.git%3Ba=commit%3Bh=76f0cad6f4e0fd...
So - I'm not entirely sure if this is a bug or expected behavior.
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.
While I agree - *I* don't have an explanation (despite digging), only work-arounds.
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-...
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`?
That's an idea - I'll see if this works.
If not, can we just use `-z execstack` rather than concatenating a DISCARD section into a linker script?
so... something like v1 patch, but replace `-z noexecstack` with `-z execstack`? And for arm64 only? I'll try this.
Either command line flags feel cleaner than modifying a linker script at build time, if they work that is.
well... that entire linker script is generated at build-time.
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@vger.kernel.org # 5.15, 5.10, 5.4 Cc: linux-kbuild@vger.kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Masahiro Yamada masahiroy@kernel.org Cc: Nicholas Piggin npiggin@gmail.com Cc: Michal Marek michal.lkml@markovi.net Cc: Nathan Chancellor nathan@kernel.org Signed-off-by: Tom Saeger tom.saeger@oracle.com
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@oracle.com/
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
On Wed, Dec 21, 2022 at 12:42 PM Tom Saeger tom.saeger@oracle.com wrote:
On Wed, Dec 21, 2022 at 11:56:33AM -0800, Nick Desaulniers wrote:
On Thu, Dec 15, 2022 at 3:18 PM Tom Saeger tom.saeger@oracle.com wrote:
v1 cover has a simple example if someone has capability/time to adapt to another architecture.
- enable CONFIG_MODVERSIONS
- build
- readelf -n vmlinux
Keep this info in the commit message.
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
I've found: https://sourceware.org/bugzilla/show_bug.cgi?id=16744 Comment 1: https://sourceware.org/bugzilla/show_bug.cgi?id=16744#c1
"the semantics of a .note.GNU-stack presence is target-dependent."
I wonder if that's an observation, or a statement of intended design. A comment in a bug tracker is perhaps less normative than explicit documentation.
Probably doesn't hurt to include that link in the commit message as well.
corresponding to this commit: https://sourceware.org/git/?p=binutils-gdb.git%3Ba=commit%3Bh=76f0cad6f4e0fd...
Seems x86 specific...
So - I'm not entirely sure if this is a bug or expected behavior.
Nick Clifton is cc'ed and might be able to provide more details (holiday timing permitting; no rush).
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.
While I agree - *I* don't have an explanation (despite digging), only work-arounds.
That's fine. That's why I'd rather have a bug on file that we link to stating we're working around this until we have a more definitive review of this surprising behavior. Please file a bug wrt. this behavior. https://sourceware.org/bugzilla/enter_bug.cgi?product=binutils
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-...
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`?
That's an idea - I'll see if this works.
Yes, please try this first.
If not, can we just use `-z execstack` rather than concatenating a DISCARD section into a linker script?
so... something like v1 patch, but replace `-z noexecstack` with `-z execstack`? And for arm64 only? I'll try this.
If --build-id doesn't work, then I'd try this. Doesn't have to be arm64 only if it's difficult to express that.
Either command line flags feel cleaner than modifying a linker script at build time, if they work that is.
well... that entire linker script is generated at build-time.
Fair, but yuck!
On Wed, Dec 21, 2022 at 01:23:40PM -0800, Nick Desaulniers wrote:
On Wed, Dec 21, 2022 at 12:42 PM Tom Saeger tom.saeger@oracle.com wrote:
On Wed, Dec 21, 2022 at 11:56:33AM -0800, Nick Desaulniers wrote:
On Thu, Dec 15, 2022 at 3:18 PM Tom Saeger tom.saeger@oracle.com wrote:
v1 cover has a simple example if someone has capability/time to adapt to another architecture.
- enable CONFIG_MODVERSIONS
- build
- readelf -n vmlinux
Keep this info in the commit message.
Ok.
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
I've found: https://sourceware.org/bugzilla/show_bug.cgi?id=16744 Comment 1: https://sourceware.org/bugzilla/show_bug.cgi?id=16744#c1
"the semantics of a .note.GNU-stack presence is target-dependent."
I wonder if that's an observation, or a statement of intended design. A comment in a bug tracker is perhaps less normative than explicit documentation.
Probably doesn't hurt to include that link in the commit message as well.
corresponding to this commit: https://sourceware.org/git/?p=binutils-gdb.git%3Ba=commit%3Bh=76f0cad6f4e0fd...
Seems x86 specific...
So - I'm not entirely sure if this is a bug or expected behavior.
Nick Clifton is cc'ed and might be able to provide more details (holiday timing permitting; no rush).
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.
While I agree - *I* don't have an explanation (despite digging), only work-arounds.
That's fine. That's why I'd rather have a bug on file that we link to stating we're working around this until we have a more definitive review of this surprising behavior. Please file a bug wrt. this behavior. https://sourceware.org/bugzilla/enter_bug.cgi?product=binutils
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-...
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`?
That's an idea - I'll see if this works.
Yes, please try this first.
--build-id=sha1 is already being supplied during link of vmlinux
If not, can we just use `-z execstack` rather than concatenating a DISCARD section into a linker script?
so... something like v1 patch, but replace `-z noexecstack` with `-z execstack`? And for arm64 only? I'll try this.
If --build-id doesn't work, then I'd try this. Doesn't have to be arm64 only if it's difficult to express that.
I went back to only trying this on arch/arm64/kernel/head.S
-z noexecstack doesn't work -z execstack also doesn't work but removing both does work.
The flow is roughly:
gcc head.S -> head.o ld -z noexecstack head.o -> .tmp_head.o mv -f .tmp_head.o head.o ld -o vmlinux --whole-archive arch/arm64/kernel/head.o ...
If I supply just the compiled head.o, not .tmp_head.o everything works.
ld of head.o with either {-z noexecstack or -z execstack} adds ".note.GNU-stack" section to the .o
This seems to be the difference.
Ideas on how to proceed?
Either command line flags feel cleaner than modifying a linker script at build time, if they work that is.
well... that entire linker script is generated at build-time.
Fair, but yuck!
Thanks, ~Nick Desaulniers
On Wed, Dec 21, 2022 at 05:54:24PM -0600, Tom Saeger wrote:
On Wed, Dec 21, 2022 at 01:23:40PM -0800, Nick Desaulniers wrote:
On Wed, Dec 21, 2022 at 12:42 PM Tom Saeger tom.saeger@oracle.com wrote:
On Wed, Dec 21, 2022 at 11:56:33AM -0800, Nick Desaulniers wrote:
On Thu, Dec 15, 2022 at 3:18 PM Tom Saeger tom.saeger@oracle.com wrote:
v1 cover has a simple example if someone has capability/time to adapt to another architecture.
- enable CONFIG_MODVERSIONS
- build
- readelf -n vmlinux
Keep this info in the commit message.
Ok.
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
I've found: https://sourceware.org/bugzilla/show_bug.cgi?id=16744 Comment 1: https://sourceware.org/bugzilla/show_bug.cgi?id=16744#c1
"the semantics of a .note.GNU-stack presence is target-dependent."
I wonder if that's an observation, or a statement of intended design. A comment in a bug tracker is perhaps less normative than explicit documentation.
Probably doesn't hurt to include that link in the commit message as well.
corresponding to this commit: https://sourceware.org/git/?p=binutils-gdb.git%3Ba=commit%3Bh=76f0cad6f4e0fd...
Seems x86 specific...
So - I'm not entirely sure if this is a bug or expected behavior.
Nick Clifton is cc'ed and might be able to provide more details (holiday timing permitting; no rush).
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.
While I agree - *I* don't have an explanation (despite digging), only work-arounds.
That's fine. That's why I'd rather have a bug on file that we link to stating we're working around this until we have a more definitive review of this surprising behavior. Please file a bug wrt. this behavior. https://sourceware.org/bugzilla/enter_bug.cgi?product=binutils
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-...
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`?
That's an idea - I'll see if this works.
Yes, please try this first.
--build-id=sha1 is already being supplied during link of vmlinux
If not, can we just use `-z execstack` rather than concatenating a DISCARD section into a linker script?
so... something like v1 patch, but replace `-z noexecstack` with `-z execstack`? And for arm64 only? I'll try this.
If --build-id doesn't work, then I'd try this. Doesn't have to be arm64 only if it's difficult to express that.
I went back to only trying this on arch/arm64/kernel/head.S
-z noexecstack doesn't work -z execstack also doesn't work but removing both does work.
The flow is roughly:
gcc head.S -> head.o ld -z noexecstack head.o -> .tmp_head.o mv -f .tmp_head.o head.o ld -o vmlinux --whole-archive arch/arm64/kernel/head.o ...
If I supply just the compiled head.o, not .tmp_head.o everything works.
Sorry, this is incorrect. ld of vmlinux actually failed.
relocation R_AARCH64_ABS32 against `__crc_kimage_vaddr' can not be used when making a shared object arch/arm64/kernel/head.o.orig: in function `__primary_switch': .../arch/arm64/kernel/head.S:897:(.idmap.text+0x458): dangerous relocation: unsupported relocation .../arch/arm64/kernel/head.S:897:(.idmap.text+0x460): dangerous relocation: unsupported relocation
ld of head.o with either {-z noexecstack or -z execstack} adds ".note.GNU-stack" section to the .o
This seems to be the difference.
Ideas on how to proceed?
Either command line flags feel cleaner than modifying a linker script at build time, if they work that is.
well... that entire linker script is generated at build-time.
Fair, but yuck!
Thanks, ~Nick Desaulniers
On Wed, Dec 21, 2022 at 06:03:30PM -0600, Tom Saeger wrote:
On Wed, Dec 21, 2022 at 05:54:24PM -0600, Tom Saeger wrote:
On Wed, Dec 21, 2022 at 01:23:40PM -0800, Nick Desaulniers wrote:
On Wed, Dec 21, 2022 at 12:42 PM Tom Saeger tom.saeger@oracle.com wrote:
On Wed, Dec 21, 2022 at 11:56:33AM -0800, Nick Desaulniers wrote:
On Thu, Dec 15, 2022 at 3:18 PM Tom Saeger tom.saeger@oracle.com wrote:
v1 cover has a simple example if someone has capability/time to adapt to another architecture.
- enable CONFIG_MODVERSIONS
- build
- readelf -n vmlinux
Keep this info in the commit message.
Ok.
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
I've found: https://sourceware.org/bugzilla/show_bug.cgi?id=16744 Comment 1: https://sourceware.org/bugzilla/show_bug.cgi?id=16744#c1
"the semantics of a .note.GNU-stack presence is target-dependent."
I wonder if that's an observation, or a statement of intended design. A comment in a bug tracker is perhaps less normative than explicit documentation.
Probably doesn't hurt to include that link in the commit message as well.
corresponding to this commit: https://sourceware.org/git/?p=binutils-gdb.git%3Ba=commit%3Bh=76f0cad6f4e0fd...
Seems x86 specific...
So - I'm not entirely sure if this is a bug or expected behavior.
Nick Clifton is cc'ed and might be able to provide more details (holiday timing permitting; no rush).
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.
While I agree - *I* don't have an explanation (despite digging), only work-arounds.
That's fine. That's why I'd rather have a bug on file that we link to stating we're working around this until we have a more definitive review of this surprising behavior. Please file a bug wrt. this behavior. https://sourceware.org/bugzilla/enter_bug.cgi?product=binutils
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-...
> 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`?
That's an idea - I'll see if this works.
Yes, please try this first.
--build-id=sha1 is already being supplied during link of vmlinux
If not, can we just use `-z execstack` rather than concatenating a DISCARD section into a linker script?
so... something like v1 patch, but replace `-z noexecstack` with `-z execstack`? And for arm64 only? I'll try this.
If --build-id doesn't work, then I'd try this. Doesn't have to be arm64 only if it's difficult to express that.
I went back to only trying this on arch/arm64/kernel/head.S
-z noexecstack doesn't work -z execstack also doesn't work but removing both does work.
The flow is roughly:
gcc head.S -> head.o ld -z noexecstack head.o -> .tmp_head.o mv -f .tmp_head.o head.o ld -o vmlinux --whole-archive arch/arm64/kernel/head.o ...
If I supply just the compiled head.o, not .tmp_head.o everything works.
Sorry, this is incorrect. ld of vmlinux actually failed.
relocation R_AARCH64_ABS32 against `__crc_kimage_vaddr' can not be used when making a shared object arch/arm64/kernel/head.o.orig: in function `__primary_switch': .../arch/arm64/kernel/head.S:897:(.idmap.text+0x458): dangerous relocation: unsupported relocation .../arch/arm64/kernel/head.S:897:(.idmap.text+0x460): dangerous relocation: unsupported relocation
Ok, I'm confused and don't know what to do here. I'll drop this from my mbox queue and wait for a revised fix to show up.
thanks,
greg k-h
On Sat, Dec 31, 2022 at 12:53:03PM +0100, Greg Kroah-Hartman wrote:
On Wed, Dec 21, 2022 at 06:03:30PM -0600, Tom Saeger wrote:
On Wed, Dec 21, 2022 at 05:54:24PM -0600, Tom Saeger wrote:
On Wed, Dec 21, 2022 at 01:23:40PM -0800, Nick Desaulniers wrote:
On Wed, Dec 21, 2022 at 12:42 PM Tom Saeger tom.saeger@oracle.com wrote:
On Wed, Dec 21, 2022 at 11:56:33AM -0800, Nick Desaulniers wrote:
On Thu, Dec 15, 2022 at 3:18 PM Tom Saeger tom.saeger@oracle.com wrote: >
v1 cover has a simple example if someone has capability/time to adapt to another architecture.
- enable CONFIG_MODVERSIONS
- build
- readelf -n vmlinux
Keep this info in the commit message.
Ok.
> > 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
I've found: https://sourceware.org/bugzilla/show_bug.cgi?id=16744 Comment 1: https://sourceware.org/bugzilla/show_bug.cgi?id=16744#c1
"the semantics of a .note.GNU-stack presence is target-dependent."
I wonder if that's an observation, or a statement of intended design. A comment in a bug tracker is perhaps less normative than explicit documentation.
Probably doesn't hurt to include that link in the commit message as well.
corresponding to this commit: https://sourceware.org/git/?p=binutils-gdb.git%3Ba=commit%3Bh=76f0cad6f4e0fd...
Seems x86 specific...
So - I'm not entirely sure if this is a bug or expected behavior.
Nick Clifton is cc'ed and might be able to provide more details (holiday timing permitting; no rush).
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.
While I agree - *I* don't have an explanation (despite digging), only work-arounds.
That's fine. That's why I'd rather have a bug on file that we link to stating we're working around this until we have a more definitive review of this surprising behavior. Please file a bug wrt. this behavior. https://sourceware.org/bugzilla/enter_bug.cgi?product=binutils
> > 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-... >> 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`?
That's an idea - I'll see if this works.
Yes, please try this first.
--build-id=sha1 is already being supplied during link of vmlinux
If not, can we just use `-z execstack` rather than concatenating a DISCARD section into a linker script?
so... something like v1 patch, but replace `-z noexecstack` with `-z execstack`? And for arm64 only? I'll try this.
If --build-id doesn't work, then I'd try this. Doesn't have to be arm64 only if it's difficult to express that.
I went back to only trying this on arch/arm64/kernel/head.S
-z noexecstack doesn't work -z execstack also doesn't work but removing both does work.
The flow is roughly:
gcc head.S -> head.o ld -z noexecstack head.o -> .tmp_head.o mv -f .tmp_head.o head.o ld -o vmlinux --whole-archive arch/arm64/kernel/head.o ...
If I supply just the compiled head.o, not .tmp_head.o everything works.
Sorry, this is incorrect. ld of vmlinux actually failed.
relocation R_AARCH64_ABS32 against `__crc_kimage_vaddr' can not be used when making a shared object arch/arm64/kernel/head.o.orig: in function `__primary_switch': .../arch/arm64/kernel/head.S:897:(.idmap.text+0x458): dangerous relocation: unsupported relocation .../arch/arm64/kernel/head.S:897:(.idmap.text+0x460): dangerous relocation: unsupported relocation
Ok, I'm confused and don't know what to do here. I'll drop this from my mbox queue and wait for a revised fix to show up.
I'm actively looking at this, but running low on things to try next.
--Tom
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Masahiro Yamada -
Nick Desaulniers -
Tom Saeger