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

[Tom Saeger <tom.saeger@oracle.com>]

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