From: Ard Biesheuvel <ardb@kernel.org>

Pre-existing code uses a dot prefix or double underscore to prefix ELF
section names. strip_relocs on x86 relies on this, and other out of tree
tools that mangle vmlinux (kexec or live patching) may rely on this as
well.

So let's not deviate from this and use a dot prefix for runtime-const
and alloc_tags sections.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Kent Overstreet <kent.overstreet@linux.dev>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arch@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-kbuild@vger.kernel.org

Ard Biesheuvel (2):
  codetag: Use dot prefix for section name
  runtime-const: Use dot prefix for section names

 arch/arm64/include/asm/runtime-const.h | 4 ++--
 arch/s390/include/asm/runtime-const.h  | 4 ++--
 arch/x86/include/asm/runtime-const.h   | 4 ++--
 include/asm-generic/codetag.lds.h      | 2 +-
 include/asm-generic/vmlinux.lds.h      | 4 ++--
 include/linux/alloc_tag.h              | 4 ++--
 6 files changed, 11 insertions(+), 11 deletions(-)

-- 
2.47.0.rc1.288.g06298d1525-goog

On Mon, 14 Oct 2024 at 05:57, Ard Biesheuvel <ardb+git@google.com> wrote:
>
> Pre-existing code uses a dot prefix or double underscore to prefix ELF
> section names. strip_relocs on x86 relies on this, and other out of tree
> tools that mangle vmlinux (kexec or live patching) may rely on this as
> well.
>
> So let's not deviate from this and use a dot prefix for runtime-const
> and alloc_tags sections.

I'm not following what the actual problem is. Yes, I see that you
report that it results in section names like ".relaalloc_tags", but
what's the actual _issue_ with that? It seems entirely harmless.

In fact, when I was going the runtime sections, I was thinking how
convenient it was for the linker to generate the start/stop symbols
for us, and that we should perhaps *expand* on that pattern.

So this seems a step backwards to me, with no real explanation of what
the actual problem is.

Yes, we have (two different) pre-existing patterns, but neither
pattern seems to be an actual improvement.

           Linus

On Mon, 14 Oct 2024 at 19:29, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, 14 Oct 2024 at 05:57, Ard Biesheuvel <ardb+git@google.com> wrote:
> >
> > Pre-existing code uses a dot prefix or double underscore to prefix ELF
> > section names. strip_relocs on x86 relies on this, and other out of tree
> > tools that mangle vmlinux (kexec or live patching) may rely on this as
> > well.
> >
> > So let's not deviate from this and use a dot prefix for runtime-const
> > and alloc_tags sections.
>
> I'm not following what the actual problem is. Yes, I see that you
> report that it results in section names like ".relaalloc_tags", but
> what's the actual _issue_ with that? It seems entirely harmless.
>
> In fact, when I was going the runtime sections, I was thinking how
> convenient it was for the linker to generate the start/stop symbols
> for us, and that we should perhaps *expand* on that pattern.
>
> So this seems a step backwards to me, with no real explanation of what
> the actual problem is.
>
> Yes, we have (two different) pre-existing patterns, but neither
> pattern seems to be an actual improvement.
>

We have this code in arch/x86/Makefile.postlink:

quiet_cmd_strip_relocs = RSTRIP  $@
      cmd_strip_relocs = \
        $(OBJCOPY) --remove-section='.rel.*' --remove-section='.rel__*' \
                   --remove-section='.rela.*' --remove-section='.rela__*' $@

Of course, that could easily be fixed, I was just being cautious in
case there is other, out-of-tree tooling for live patch or kexec etc
that has similar assumptions wrt section names.

On Mon, 14 Oct 2024 at 10:44, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> We have this code in arch/x86/Makefile.postlink:
>
> quiet_cmd_strip_relocs = RSTRIP  $@
>       cmd_strip_relocs = \
>         $(OBJCOPY) --remove-section='.rel.*' --remove-section='.rel__*' \
>                    --remove-section='.rela.*' --remove-section='.rela__*' $@
>
> Of course, that could easily be fixed, I was just being cautious in
> case there is other, out-of-tree tooling for live patch or kexec etc
> that has similar assumptions wrt section names.

I'd actually much rather just make strip_relocs not have that "." and
"__" pattern at all, and just say "we strip all sections that start
with '.rel'".

And then we make the rule that we do *not* create sections named ".rel*".

That seems like a much simpler rule, and would seem to simplify
strip_relocs too, which would just become

        $(OBJCOPY) --remove-section='.rel*' $@

(We seem to have three different copies of that complex pattern with
.rel vs .rela and "." vs "__" - it's in s390, riscv, and x86. So we'd
do that simplification in three places)

IOW, I'd much rather make our section rules simpler rather than more complex.

Of course, if there is some active and acute problem report with this
thing, we might not have that option, but in the absence of any
*known* issue with just simplifying things, I'd rather do that.

I feel that our linker scripts - and linking rules in general - are
already quite complicated, which is why I'd really like to take this
as a time to try to simplify the rules.

              Linus

              Linus

On Mon, 14 Oct 2024 at 20:10, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, 14 Oct 2024 at 10:44, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > We have this code in arch/x86/Makefile.postlink:
> >
> > quiet_cmd_strip_relocs = RSTRIP  $@
> >       cmd_strip_relocs = \
> >         $(OBJCOPY) --remove-section='.rel.*' --remove-section='.rel__*' \
> >                    --remove-section='.rela.*' --remove-section='.rela__*' $@
> >
> > Of course, that could easily be fixed, I was just being cautious in
> > case there is other, out-of-tree tooling for live patch or kexec etc
> > that has similar assumptions wrt section names.
>
> I'd actually much rather just make strip_relocs not have that "." and
> "__" pattern at all, and just say "we strip all sections that start
> with '.rel'".
>
> And then we make the rule that we do *not* create sections named ".rel*".
>
> That seems like a much simpler rule, and would seem to simplify
> strip_relocs too, which would just become
>
>         $(OBJCOPY) --remove-section='.rel*' $@
>
> (We seem to have three different copies of that complex pattern with
> .rel vs .rela and "." vs "__" - it's in s390, riscv, and x86. So we'd
> do that simplification in three places)
>
> IOW, I'd much rather make our section rules simpler rather than more complex.
>
> Of course, if there is some active and acute problem report with this
> thing, we might not have that option, but in the absence of any
> *known* issue with just simplifying things, I'd rather do that.
>

I don't disagree with any of this. CC'ing folks working on live patch
in case they have any insights.

Full thread here:
https://lore.kernel.org/all/20241014125703.2287936-4-ardb+git@google.com/

On Mon 2024-10-14 20:19:32, Ard Biesheuvel wrote:
> On Mon, 14 Oct 2024 at 20:10, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > On Mon, 14 Oct 2024 at 10:44, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > We have this code in arch/x86/Makefile.postlink:
> > >
> > > quiet_cmd_strip_relocs = RSTRIP  $@
> > >       cmd_strip_relocs = \
> > >         $(OBJCOPY) --remove-section='.rel.*' --remove-section='.rel__*' \
> > >                    --remove-section='.rela.*' --remove-section='.rela__*' $@
> > >
> > > Of course, that could easily be fixed, I was just being cautious in
> > > case there is other, out-of-tree tooling for live patch or kexec etc
> > > that has similar assumptions wrt section names.
> >
> > I'd actually much rather just make strip_relocs not have that "." and
> > "__" pattern at all, and just say "we strip all sections that start
> > with '.rel'".
> >
> > And then we make the rule that we do *not* create sections named ".rel*".
> >
> > That seems like a much simpler rule, and would seem to simplify
> > strip_relocs too, which would just become
> >
> >         $(OBJCOPY) --remove-section='.rel*' $@
> >
> > (We seem to have three different copies of that complex pattern with
> > .rel vs .rela and "." vs "__" - it's in s390, riscv, and x86. So we'd
> > do that simplification in three places)
> >
> > IOW, I'd much rather make our section rules simpler rather than more complex.
> >
> > Of course, if there is some active and acute problem report with this
> > thing, we might not have that option, but in the absence of any
> > *known* issue with just simplifying things, I'd rather do that.
> >
> 
> I don't disagree with any of this. CC'ing folks working on live patch
> in case they have any insights.

The livepatching specific sections use the name pattern:

	.klp.rela.objname.section_name

They are generated by a post processing of the livepatch module.
The code is not upstream at the moment. It is implemented by
some out-of-tree tools which are used to build the livepatches.

More details can be found in
Documentation/livepatch/module-elf-format.rst

Best Regards,
Petr