Re: [PATCH] x86/build: Simplify patterns for unwanted section

[Kees Cook <keescook@chromium.org>]

On Wed, Feb 14, 2024 at 02:13:01PM -0800, Fangrui Song wrote:
> On Wed, Feb 14, 2024 at 2:07 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Wed, Feb 14, 2024 at 01:29:29PM -0800, Fangrui Song wrote:
> > > A s390 patch modeling its --orphan-handling= after x86 [1] sparked my
> > > motivation to simplify patterns. Commit 5354e84598f2 ("x86/build: Add
> > > asserts for unwanted sections") added asserts that certain input
> > > sections must be absent or empty. The patterns can be simplified.
> > >
> > > For dynamic relocations,
> > >
> > > *(.rela.*) is sufficient to match all dynamic relocations synthesized by
> > > GNU ld and LLD. .rela_* is unnecessary. --emit-relocs may create .rela_*
> > > sections for section names prefixed with _, but they are not matched by
> > > linker scripts.
> > >
> > > .plt instead of .plt.* is sufficient to match synthesized PLT entries.
> >
> > Do you mean ".plt.foo" matches ".plt" ?
> 
> I mean we just need .plt : { *(.plt) } , not .plt : { *(.plt) *(.plt.*) }.

But then, for example, if it gets generated, .plt.got ends up being
reported as an orphan...

> 
> The linker synthesized section for PLT entries is .plt, not suffixed.
> 
> > > .igot and .igot.plt are for non-preemptible STT_GNU_IFUNC in GNU ld (LLD
> > > just uses .got), which the kernel does not use. In addition, if .igot or

Right, the issue has been getting totally weird sections emitted by the
linker. If you're saying you'd rather those get reported as orphan
sections instead of being validated for being zero sized, and that works
for all the architectures, then okay.

> > > .igot.plt is ever non-empty, there will be .rela.* dynamic relocations
> > > leading to an assert failure anyway.
> >
> > I think at the time I was dealing with avoid multiple warnings out of
> > the linker, as I was getting orphan warnings in addition to the
> > non-empty warnings.
> >
> > >
> > > [1]: https://lore.kernel.org/all/20240207-s390-lld-and-orphan-warn-v1-6-8a665b3346ab@kernel.org/
> > >
> > > Signed-off-by: Fangrui Song <maskray@google.com>
> >
> > Is anything harmed by leaving all of this as-is?
> >
> > -Kees
> 
> No harm. But ports adopting --orphan-handling= (like s390) may copy
> the unneeded .rela_* .
> When people read .rela_*, they might think whether the kernel does
> anything special that
> .rela_* needs to be matched.

I added these because the were being generated. See commit d1c0272bc1c0
("x86/boot/compressed: Remove, discard, or assert for unwanted sections")

I don't want to suddenly start generating warnings for older/broken
linkers. (i.e. a change like this needs really careful testing, and that
needs to be detailed in the commit log.)

-Kees

-- 
Kees Cook