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

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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