[PATCH 0/2] Use dot prefixes for section names

[PATCH 0/2] Use dot prefixes for section names

[Ard Biesheuvel]

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

[PATCH 1/2] codetag: Use dot prefix for section name

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Sections typically use leading dots in their names, and deviating from
this breaks some assumptions in the existing code, e.g., in strip_relocs
on x86, which filters out .rela.* and .rela__* sections.

  [65] alloc_tags        PROGBITS         0000000000000000  03a57958
       0000000000026340  0000000000000000  WA       0     0     8
  [66] .relaalloc_tags   RELA             0000000000000000  08dbb868
       0000000000044c40  0000000000000018   I      280    65     8

So use a leading dot for the alloc_tags sections.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 include/asm-generic/codetag.lds.h | 2 +-
 include/linux/alloc_tag.h         | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/asm-generic/codetag.lds.h b/include/asm-generic/codetag.lds.h
index 64f536b80380..dcd18351ba2f 100644
--- a/include/asm-generic/codetag.lds.h
+++ b/include/asm-generic/codetag.lds.h
@@ -5,7 +5,7 @@
 #define SECTION_WITH_BOUNDARIES(_name)	\
 	. = ALIGN(8);			\
 	__start_##_name = .;		\
-	KEEP(*(_name))			\
+	KEEP(*(. ## _name))		\
 	__stop_##_name = .;
 
 #define CODETAG_SECTIONS()		\
diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
index 1f0a9ff23a2c..d45a8a582970 100644
--- a/include/linux/alloc_tag.h
+++ b/include/linux/alloc_tag.h
@@ -76,7 +76,7 @@ DECLARE_PER_CPU(struct alloc_tag_counters, _shared_alloc_tag);
 
 #define DEFINE_ALLOC_TAG(_alloc_tag)						\
 	static struct alloc_tag _alloc_tag __used __aligned(8)			\
-	__section("alloc_tags") = {						\
+	__section(".alloc_tags") = {						\
 		.ct = CODE_TAG_INIT,						\
 		.counters = &_shared_alloc_tag };
 
@@ -85,7 +85,7 @@ DECLARE_PER_CPU(struct alloc_tag_counters, _shared_alloc_tag);
 #define DEFINE_ALLOC_TAG(_alloc_tag)						\
 	static DEFINE_PER_CPU(struct alloc_tag_counters, _alloc_tag_cntr);	\
 	static struct alloc_tag _alloc_tag __used __aligned(8)			\
-	__section("alloc_tags") = {						\
+	__section(".alloc_tags") = {						\
 		.ct = CODE_TAG_INIT,						\
 		.counters = &_alloc_tag_cntr };
 
-- 
2.47.0.rc1.288.g06298d1525-goog

[PATCH 2/2] runtime-const: Use dot prefix for section names

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Sections typically use leading dots in their names, and deviating from
this breaks some assumptions in the existing code, e.g., in strip_relocs
on x86, as the linker prepends .rela to the section names when emitting
static relocations.

  [59] .relaruntime_ptr_dentry_hashtable RELA            0000000000000000 792c758 000078 18   I 56  38  8
  [60] .relaruntime_shift_d_hash_shift RELA            0000000000000000 792c7d0 000078 18   I 56  37  8

So use a leading dot for the runtime const sections.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 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/vmlinux.lds.h      | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/runtime-const.h b/arch/arm64/include/asm/runtime-const.h
index be5915669d23..2c3521765cfe 100644
--- a/arch/arm64/include/asm/runtime-const.h
+++ b/arch/arm64/include/asm/runtime-const.h
@@ -14,7 +14,7 @@
 		"movk %0, #0x89ab, lsl #16\n\t"			\
 		"movk %0, #0x4567, lsl #32\n\t"			\
 		"movk %0, #0x0123, lsl #48\n\t"			\
-		".pushsection runtime_ptr_" #sym ",\"a\"\n\t"	\
+		".pushsection .runtime_ptr_" #sym ",\"a\"\n\t"	\
 		".long 1b - .\n\t"				\
 		".popsection"					\
 		:"=r" (__ret));					\
@@ -24,7 +24,7 @@
 	unsigned long __ret;					\
 	asm_inline("1:\t"					\
 		"lsr %w0,%w1,#12\n\t"				\
-		".pushsection runtime_shift_" #sym ",\"a\"\n\t"	\
+		".pushsection .runtime_shift_" #sym ",\"a\"\n\t"\
 		".long 1b - .\n\t"				\
 		".popsection"					\
 		:"=r" (__ret)					\
diff --git a/arch/s390/include/asm/runtime-const.h b/arch/s390/include/asm/runtime-const.h
index 17878b1d048c..d19f54567d48 100644
--- a/arch/s390/include/asm/runtime-const.h
+++ b/arch/s390/include/asm/runtime-const.h
@@ -11,7 +11,7 @@
 	asm_inline(						\
 		"0:	iihf	%[__ret],%[c1]\n"		\
 		"	iilf	%[__ret],%[c2]\n"		\
-		".pushsection runtime_ptr_" #sym ",\"a\"\n"	\
+		".pushsection .runtime_ptr_" #sym ",\"a\"\n"	\
 		".long 0b - .\n"				\
 		".popsection"					\
 		: [__ret] "=d" (__ret)				\
@@ -26,7 +26,7 @@
 								\
 	asm_inline(						\
 		"0:	srl	%[__ret],12\n"			\
-		".pushsection runtime_shift_" #sym ",\"a\"\n"	\
+		".pushsection .runtime_shift_" #sym ",\"a\"\n"	\
 		".long 0b - .\n"				\
 		".popsection"					\
 		: [__ret] "+d" (__ret));			\
diff --git a/arch/x86/include/asm/runtime-const.h b/arch/x86/include/asm/runtime-const.h
index 24e3a53ca255..0de5a40ee6d0 100644
--- a/arch/x86/include/asm/runtime-const.h
+++ b/arch/x86/include/asm/runtime-const.h
@@ -5,7 +5,7 @@
 #define runtime_const_ptr(sym) ({				\
 	typeof(sym) __ret;					\
 	asm_inline("mov %1,%0\n1:\n"				\
-		".pushsection runtime_ptr_" #sym ",\"a\"\n\t"	\
+		".pushsection .runtime_ptr_" #sym ",\"a\"\n\t"	\
 		".long 1b - %c2 - .\n\t"			\
 		".popsection"					\
 		:"=r" (__ret)					\
@@ -19,7 +19,7 @@
 #define runtime_const_shift_right_32(val, sym) ({		\
 	typeof(0u+(val)) __ret = (val);				\
 	asm_inline("shrl $12,%k0\n1:\n"				\
-		".pushsection runtime_shift_" #sym ",\"a\"\n\t"	\
+		".pushsection .runtime_shift_" #sym ",\"a\"\n\t"\
 		".long 1b - 1 - .\n\t"				\
 		".popsection"					\
 		:"+r" (__ret));					\
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index eeadbaeccf88..da097ba2d4d8 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -914,8 +914,8 @@
 
 #define NAMED_SECTION(name) \
 	. = ALIGN(8); \
-	name : AT(ADDR(name) - LOAD_OFFSET) \
-	{ BOUNDED_SECTION_PRE_LABEL(name, name, __start_, __stop_) }
+	. ## name : AT(ADDR(. ## name) - LOAD_OFFSET) \
+	{ BOUNDED_SECTION_PRE_LABEL(. ## name, name, __start_, __stop_) }
 
 #define RUNTIME_CONST(t,x) NAMED_SECTION(runtime_##t##_##x)
 
-- 
2.47.0.rc1.288.g06298d1525-goog

Re: [PATCH 0/2] Use dot prefixes for section names

[Linus Torvalds]

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

Re: [PATCH 0/2] Use dot prefixes for section names

[Ard Biesheuvel]

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.

Re: [PATCH 0/2] Use dot prefixes for section names

[Linus Torvalds]

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

Re: [PATCH 0/2] Use dot prefixes for section names

[Ard Biesheuvel]

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/

Re: [PATCH 1/2] codetag: Use dot prefix for section name

[Suren Baghdasaryan]

On Mon, Oct 14, 2024 at 5:57 AM Ard Biesheuvel <ardb+git@google.com> wrote:
>
> From: Ard Biesheuvel <ardb@kernel.org>
>
> Sections typically use leading dots in their names, and deviating from
> this breaks some assumptions in the existing code, e.g., in strip_relocs
> on x86, which filters out .rela.* and .rela__* sections.
>
>   [65] alloc_tags        PROGBITS         0000000000000000  03a57958
>        0000000000026340  0000000000000000  WA       0     0     8
>   [66] .relaalloc_tags   RELA             0000000000000000  08dbb868
>        0000000000044c40  0000000000000018   I      280    65     8
>
> So use a leading dot for the alloc_tags sections.

No issues with renaming the section but please note that I posted a
patch [1] today that will have conflicts with this renaming.

[1] https://lore.kernel.org/all/20241014203646.1952505-3-surenb@google.com/

>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  include/asm-generic/codetag.lds.h | 2 +-
>  include/linux/alloc_tag.h         | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/asm-generic/codetag.lds.h b/include/asm-generic/codetag.lds.h
> index 64f536b80380..dcd18351ba2f 100644
> --- a/include/asm-generic/codetag.lds.h
> +++ b/include/asm-generic/codetag.lds.h
> @@ -5,7 +5,7 @@
>  #define SECTION_WITH_BOUNDARIES(_name) \
>         . = ALIGN(8);                   \
>         __start_##_name = .;            \
> -       KEEP(*(_name))                  \
> +       KEEP(*(. ## _name))             \
>         __stop_##_name = .;

I think leaving the SECTION_WITH_BOUNDARIES() definition as is and
changing its users to pass the name with the dot would be more
explicit and more flexible.
The only user today is CODETAG_SECTIONS() at [2], so I would add the
dot in there instead.

[2] https://elixir.bootlin.com/linux/v6.12-rc2/source/include/asm-generic/codetag.lds.h#L12

Thanks,
Suren.

>
>  #define CODETAG_SECTIONS()             \
> diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
> index 1f0a9ff23a2c..d45a8a582970 100644
> --- a/include/linux/alloc_tag.h
> +++ b/include/linux/alloc_tag.h
> @@ -76,7 +76,7 @@ DECLARE_PER_CPU(struct alloc_tag_counters, _shared_alloc_tag);
>
>  #define DEFINE_ALLOC_TAG(_alloc_tag)                                           \
>         static struct alloc_tag _alloc_tag __used __aligned(8)                  \
> -       __section("alloc_tags") = {                                             \
> +       __section(".alloc_tags") = {                                            \
>                 .ct = CODE_TAG_INIT,                                            \
>                 .counters = &_shared_alloc_tag };
>
> @@ -85,7 +85,7 @@ DECLARE_PER_CPU(struct alloc_tag_counters, _shared_alloc_tag);
>  #define DEFINE_ALLOC_TAG(_alloc_tag)                                           \
>         static DEFINE_PER_CPU(struct alloc_tag_counters, _alloc_tag_cntr);      \
>         static struct alloc_tag _alloc_tag __used __aligned(8)                  \
> -       __section("alloc_tags") = {                                             \
> +       __section(".alloc_tags") = {                                            \
>                 .ct = CODE_TAG_INIT,                                            \
>                 .counters = &_alloc_tag_cntr };
>
> --
> 2.47.0.rc1.288.g06298d1525-goog
>

Re: [PATCH 0/2] Use dot prefixes for section names

[Petr Mladek]

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