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 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