On Wed, Feb 14, 2024 at 02:13:01PM -0800, Fangrui Song wrote:
> On Wed, Feb 14, 2024 at 2:07 PM Kees Cook <[email protected]> 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/[email protected]/
> > >
> > > Signed-off-by: Fangrui Song <[email protected]>
> >
> > 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
On Wed, Feb 14, 2024 at 2:40 PM Kees Cook <[email protected]> wrote:
>
> On Wed, Feb 14, 2024 at 02:13:01PM -0800, Fangrui Song wrote:
> > On Wed, Feb 14, 2024 at 2:07 PM Kees Cook <[email protected]> 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.
Thanks. I trust my judgement here:)
> > > > .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/[email protected]/
> > > >
> > > > Signed-off-by: Fangrui Song <[email protected]>
> > >
> > > 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
I saw this commit and still believe .rela_* is unnecessary for all
supported binutils versions.
GNU ld 2.25 does not support --orphan-handling=error (2015-09
feature), but it manages to link vmlinux without any warning.
/tmp/binutils-2.25/out/release/ld/ld-new -m elf_x86_64 -z noexecstack
--emit-relocs --discard-none -z max-page-size=0x200000 --build-id=sha1
--script=./arch/x86/kernel/vmlinux.lds -o vmlinux --whole-archive
vmlinux.o .vmlinux.export.o init/version-timestamp.o
--no-whole-archive --start-group --end-group .tmp_vmlinux.kallsyms2.o
> --
> Kees Cook
--
宋方睿