linux-kernel - Re: [PATCH] vmlinux.lds.S: Explicitly set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK7LNAQ_Si3feoaHujiWF0nHCu_kqb=3rrQf=OX1CoTDdZtjVA@mail.gmail.com>
Date: Wed, 13 Nov 2024 10:39:37 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Rong Xu <xur@...gle.com>
Cc: Thomas Bogendoerfer <tsbogend@...ha.franken.de>, Nick Desaulniers <ndesaulniers@...gle.com>, 
	Klara Modin <klarasmodin@...il.com>, linux-mips@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vmlinux.lds.S: Explicitly set _stext for mips

Please use "MIPS:" for the subject prefix
because this patch only affects MIPS.



On Wed, Nov 13, 2024 at 5:08 AM Rong Xu <xur@...gle.com> wrote:
>
> With commit 622240ea8d71 ("vmlinux.lds.h: Adjust symbol ordering in
> text output section"), symobls in sections .text.unlikely will be
> placed before symbols in .text. This can lead to the misplacement
> of _stext, which resides in the .text section, and result in a boot
> hang.
>
> Explicitly set _stext to the beginning of text output section.


I will insert this patch before 622240ea8d71 to avoid breakage.

Please rephrase the commit description without referring to
622240ea8d71.


For example, you can say as follows:

------------->-----------
MIPS: move _stext definition to vmlinux.lds.S

The _stext symbol is intended to reference the start of the text section.
However, it currently relies on a fragile link order because the existing
EXPORT(_stext) resides within the .text section, which is not guaranteed
to be placed first.

Move the _stext definition to the linker script to enforce an explicit
ordering.
------------->-----------


Please feel free to update the description, but this must be
fixed regardless of your patch set.

Even without your patch set, the .text section
comes after .text.hot. So, it is broken.

#define TEXT_TEXT                                                       \
                ALIGN_FUNCTION();                                       \
                *(.text.hot .text.hot.*)                                \
                *(TEXT_MAIN .text.fixup)                                \




> Signed-off-by: Rong Xu <xur@...gle.com>
> Reported-by: Klara Modin <klarasmodin@...il.com>
> Tested-by: Klara Modin <klarasmodin@...il.com>
> ---
>  arch/mips/kernel/vmlinux.lds.S | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
> index 9ff55cb80a64..62c3db869a18 100644
> --- a/arch/mips/kernel/vmlinux.lds.S
> +++ b/arch/mips/kernel/vmlinux.lds.S
> @@ -61,6 +61,11 @@ SECTIONS
>         /* read-only */
>         _text = .;      /* Text and read-only data */
>         .text : {
> +               /* Explicitly set _stext to the beginning of text output
> +                  section. _stext is in text section and might be matched
> +                  after symbols in sections with a specific prefix (like
> +                  .text.unlikely).  */

I do not think this comment is necessary
because this is a common pattern.

The typical linker script example is documented.
https://github.com/torvalds/linux/blob/v6.11-rc7/include/asm-generic/vmlinux.lds.h#L21




Many architectures define _stext in vmlinux.lds.S
without such verbose comments.

$ find . -name vmlinux.lds.S | xargs grep "_stext ="
./arch/mips/kernel/vmlinux.lds.S: _stext = . ;
./arch/openrisc/kernel/vmlinux.lds.S:          _stext = .;
./arch/xtensa/kernel/vmlinux.lds.S:  _stext = .;
./arch/s390/kernel/vmlinux.lds.S: _stext = .; /* Start of text section */
./arch/nios2/kernel/vmlinux.lds.S: _stext = .;
./arch/loongarch/kernel/vmlinux.lds.S: _stext = .;
./arch/x86/kernel/vmlinux.lds.S: _stext = .;
./arch/riscv/kernel/vmlinux.lds.S: _stext = .;
./arch/parisc/kernel/vmlinux.lds.S: _stext = .;
./arch/csky/kernel/vmlinux.lds.S: _stext = .;
./arch/arc/kernel/vmlinux.lds.S: _stext = .;
./arch/arm64/kernel/vmlinux.lds.S: _stext = .; /* Text and read-only data */
./arch/arm/kernel/vmlinux.lds.S: _stext = .; /* Text and read-only data */
./arch/hexagon/kernel/vmlinux.lds.S: _stext = .;
./arch/powerpc/kernel/vmlinux.lds.S: _stext = .;
./arch/microblaze/kernel/vmlinux.lds.S: _stext = . ;




> +               _stext = .;

_etext is defined outside the .text {} block.

If you want "_stext" and "_etext" to look symmetrical,
perhaps you might want to change like this?


diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
index 9ff55cb80a64..d575f945d422 100644
--- a/arch/mips/kernel/vmlinux.lds.S
+++ b/arch/mips/kernel/vmlinux.lds.S
@@ -60,6 +60,7 @@ SECTIONS
        . = LINKER_LOAD_ADDRESS;
        /* read-only */
        _text = .;      /* Text and read-only data */
+       _stext = .;
        .text : {
                TEXT_TEXT
                SCHED_TEXT




If you move the _stext definition to the linker script,
you can remove EXPORT(_stext) from
arch/mips/kernel/head.S







--
Best Regards
Masahiro Yamada