diff mbox series

[5.15,5.10,5.4,v2] kbuild: fix Build ID if CONFIG_MODVERSIONS

Message ID 3df32572ec7016e783d37e185f88495831671f5d.1671143628.git.tom.saeger@oracle.com (mailing list archive)
State New, archived
Headers show
Series [5.15,5.10,5.4,v2] kbuild: fix Build ID if CONFIG_MODVERSIONS | expand

Commit Message

Tom Saeger Dec. 15, 2022, 11:18 p.m. UTC 
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(+)


base-commit: fd6d66840b4269da4e90e1ea807ae3197433bc66

Comments

Greg KH Dec. 21, 2022, 4:31 p.m. UTC | #1 
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); 					\
>  										\
>  		$(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@ 		\
>  			-T $(@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
Nick Desaulniers Dec. 21, 2022, 7:56 p.m. UTC | #2 
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-segments
>> missing .note.GNU-stack section implies executable stack
The intent of 0d362be5b142 is that we don't want translation units to
have executable stacks, though I do note that assembler sources need
to opt in.

Is it possible to force a build-id via linker flag `--build-id=sha1`?

If not, can we just use `-z execstack` rather than concatenating a
DISCARD section into a linker script?  Either command line flags feel
cleaner than modifying a linker script at build time, if they work
that is.

> vmlinux then properly adds .NOTES segment containing Build ID that can
> be read using tools like 'readelf -n'.
>
> Fixes: 0d362be5b142 ("Makefile: link with -z noexecstack --no-warn-rwx-segments")
> Cc: <stable@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
>
Tom Saeger Dec. 21, 2022, 8:42 p.m. UTC | #3 
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;a=commit;h=76f0cad6f4e0fdfc4cfeee135b44b6a090919c60

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-segments
> >> missing .note.GNU-stack section implies executable stack
> The intent of 0d362be5b142 is that we don't want translation units to
> have executable stacks, though I do note that assembler sources need
> to opt in.
> 
> Is it possible to force a build-id via linker flag `--build-id=sha1`?
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
Tom Saeger Dec. 21, 2022, 8:52 p.m. UTC | #4 
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); 					\
> >  										\
> >  		$(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@ 		\
> >  			-T $(@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
Nick Desaulniers Dec. 21, 2022, 9:23 p.m. UTC | #5 
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;a=commit;h=76f0cad6f4e0fdfc4cfeee135b44b6a090919c60

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-segments
> > >> missing .note.GNU-stack section implies executable stack
> > The intent of 0d362be5b142 is that we don't want translation units to
> > have executable stacks, though I do note that assembler sources need
> > to opt in.
> >
> > Is it possible to force a build-id via linker flag `--build-id=sha1`?
> 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!
Tom Saeger Dec. 21, 2022, 11:54 p.m. UTC | #6 
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;a=commit;h=76f0cad6f4e0fdfc4cfeee135b44b6a090919c60
> 
> 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-segments
> > > >> missing .note.GNU-stack section implies executable stack
> > > The intent of 0d362be5b142 is that we don't want translation units to
> > > have executable stacks, though I do note that assembler sources need
> > > to opt in.
> > >
> > > Is it possible to force a build-id via linker flag `--build-id=sha1`?
> > 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
Tom Saeger Dec. 22, 2022, 12:03 a.m. UTC | #7 
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;a=commit;h=76f0cad6f4e0fdfc4cfeee135b44b6a090919c60
> > 
> > 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-segments
> > > > >> missing .note.GNU-stack section implies executable stack
> > > > The intent of 0d362be5b142 is that we don't want translation units to
> > > > have executable stacks, though I do note that assembler sources need
> > > > to opt in.
> > > >
> > > > Is it possible to force a build-id via linker flag `--build-id=sha1`?
> > > 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
Greg KH Dec. 22, 2022, 6:01 a.m. UTC | #8 
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
Greg KH Dec. 31, 2022, 11:53 a.m. UTC | #9 
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;a=commit;h=76f0cad6f4e0fdfc4cfeee135b44b6a090919c60
> > > 
> > > 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-segments
> > > > > >> missing .note.GNU-stack section implies executable stack
> > > > > The intent of 0d362be5b142 is that we don't want translation units to
> > > > > have executable stacks, though I do note that assembler sources need
> > > > > to opt in.
> > > > >
> > > > > Is it possible to force a build-id via linker flag `--build-id=sha1`?
> > > > 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
Tom Saeger Jan. 5, 2023, 8:30 p.m. UTC | #10 
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;a=commit;h=76f0cad6f4e0fdfc4cfeee135b44b6a090919c60
> > > > 
> > > > 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-segments
> > > > > > >> missing .note.GNU-stack section implies executable stack
> > > > > > The intent of 0d362be5b142 is that we don't want translation units to
> > > > > > have executable stacks, though I do note that assembler sources need
> > > > > > to opt in.
> > > > > >
> > > > > > Is it possible to force a build-id via linker flag `--build-id=sha1`?
> > > > > 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
Tom Saeger Jan. 9, 2023, 6:36 p.m. UTC | #11 
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
Masahiro Yamada Jan. 10, 2023, 6:58 a.m. UTC | #12 
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
Tom Saeger Jan. 10, 2023, 4:27 p.m. UTC | #13 
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
Greg KH Jan. 12, 2023, noon UTC | #14 
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
Tom Saeger Jan. 12, 2023, 9:20 p.m. UTC | #15 
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
Tom Saeger Jan. 13, 2023, 3:06 p.m. UTC | #16 
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_+jjKw@mail.gmail.com/

Regards,

--Tom
Greg KH Jan. 14, 2023, 1:53 p.m. UTC | #17 
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_+jjKw@mail.gmail.com/

Great, please let me know when this hits Linus's tree and I will be glad
to queue it up.

thanks,

greg k-h
Tom Saeger Jan. 17, 2023, 11:50 p.m. UTC | #18 
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_+jjKw@mail.gmail.com/
> 
> 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
Greg KH Jan. 18, 2023, 6:14 a.m. UTC | #19 
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
Greg KH Jan. 22, 2023, 2:21 p.m. UTC | #20 
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_+jjKw@mail.gmail.com/
> > 
> > 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
Greg KH Jan. 22, 2023, 2:40 p.m. UTC | #21 
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_+jjKw@mail.gmail.com/
> > > 
> > > 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
Greg KH Jan. 23, 2023, 7:26 a.m. UTC | #22 
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_+jjKw@mail.gmail.com/
> > > > 
> > > > 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
diff mbox series

Patch

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);				\