2022-02-18 08:31:57

by Fangrui Song

[permalink] [raw]
Subject: [PATCH] arm64 module: remove (NOLOAD)

On ELF, (NOLOAD) sets the section type to SHT_NOBITS[1]. It is conceptually
inappropriate for .plt and .text.* sections which are always
SHT_PROGBITS.

In GNU ld, if PLT entries are needed, .plt will be SHT_PROGBITS anyway
and (NOLOAD) will be essentially ignored. In ld.lld, since
https://reviews.llvm.org/D118840 ("[ELF] Support (TYPE=<value>) to
customize the output section type"), ld.lld will report a `section type
mismatch` error. Just remove (NOLOAD) to fix the error.

[1] https://lld.llvm.org/ELF/linker_script.html As of today, "The
section should be marked as not loadable" on
https://sourceware.org/binutils/docs/ld/Output-Section-Type.html is
outdated for ELF.
---
arch/arm64/include/asm/module.lds.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/module.lds.h b/arch/arm64/include/asm/module.lds.h
index a11ccadd47d2..094701ec5500 100644
--- a/arch/arm64/include/asm/module.lds.h
+++ b/arch/arm64/include/asm/module.lds.h
@@ -1,8 +1,8 @@
SECTIONS {
#ifdef CONFIG_ARM64_MODULE_PLTS
- .plt 0 (NOLOAD) : { BYTE(0) }
- .init.plt 0 (NOLOAD) : { BYTE(0) }
- .text.ftrace_trampoline 0 (NOLOAD) : { BYTE(0) }
+ .plt 0 : { BYTE(0) }
+ .init.plt 0 : { BYTE(0) }
+ .text.ftrace_trampoline 0 : { BYTE(0) }
#endif

#ifdef CONFIG_KASAN_SW_TAGS
--
2.35.1.265.g69c8d7142f-goog


2022-02-18 08:41:02

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH] arm64 module: remove (NOLOAD)

On Fri, 18 Feb 2022 at 09:12, Fangrui Song <[email protected]> wrote:
>
> On ELF, (NOLOAD) sets the section type to SHT_NOBITS[1]. It is conceptually
> inappropriate for .plt and .text.* sections which are always
> SHT_PROGBITS.
>
> In GNU ld, if PLT entries are needed, .plt will be SHT_PROGBITS anyway
> and (NOLOAD) will be essentially ignored. In ld.lld, since
> https://reviews.llvm.org/D118840 ("[ELF] Support (TYPE=<value>) to
> customize the output section type"), ld.lld will report a `section type
> mismatch` error. Just remove (NOLOAD) to fix the error.
>
> [1] https://lld.llvm.org/ELF/linker_script.html As of today, "The
> section should be marked as not loadable" on
> https://sourceware.org/binutils/docs/ld/Output-Section-Type.html is
> outdated for ELF.

This patch lacks a SOB line.

With one added,

Acked-by: Ard Biesheuvel <[email protected]>

> ---
> arch/arm64/include/asm/module.lds.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/module.lds.h b/arch/arm64/include/asm/module.lds.h
> index a11ccadd47d2..094701ec5500 100644
> --- a/arch/arm64/include/asm/module.lds.h
> +++ b/arch/arm64/include/asm/module.lds.h
> @@ -1,8 +1,8 @@
> SECTIONS {
> #ifdef CONFIG_ARM64_MODULE_PLTS
> - .plt 0 (NOLOAD) : { BYTE(0) }
> - .init.plt 0 (NOLOAD) : { BYTE(0) }
> - .text.ftrace_trampoline 0 (NOLOAD) : { BYTE(0) }
> + .plt 0 : { BYTE(0) }
> + .init.plt 0 : { BYTE(0) }
> + .text.ftrace_trampoline 0 : { BYTE(0) }
> #endif
>
> #ifdef CONFIG_KASAN_SW_TAGS
> --
> 2.35.1.265.g69c8d7142f-goog
>

2022-02-18 10:41:18

by Fangrui Song

[permalink] [raw]
Subject: Re: [PATCH] arm64 module: remove (NOLOAD)

On 2022-02-18, Ard Biesheuvel wrote:
>On Fri, 18 Feb 2022 at 09:12, Fangrui Song <[email protected]> wrote:
>>
>> On ELF, (NOLOAD) sets the section type to SHT_NOBITS[1]. It is conceptually
>> inappropriate for .plt and .text.* sections which are always
>> SHT_PROGBITS.
>>
>> In GNU ld, if PLT entries are needed, .plt will be SHT_PROGBITS anyway
>> and (NOLOAD) will be essentially ignored. In ld.lld, since
>> https://reviews.llvm.org/D118840 ("[ELF] Support (TYPE=<value>) to
>> customize the output section type"), ld.lld will report a `section type
>> mismatch` error. Just remove (NOLOAD) to fix the error.
>>
>> [1] https://lld.llvm.org/ELF/linker_script.html As of today, "The
>> section should be marked as not loadable" on
>> https://sourceware.org/binutils/docs/ld/Output-Section-Type.html is
>> outdated for ELF.
>
>This patch lacks a SOB line.
>
>With one added,
>
>Acked-by: Ard Biesheuvel <[email protected]>

Ah, yes. Sorry, I haven't sent a kernel patch for a while...

Reported-by: Nathan Chancellor <[email protected]>
Signed-off-by: Fangrui Song <[email protected]>
Acked-by: Ard Biesheuvel <[email protected]>

>> ---
>> arch/arm64/include/asm/module.lds.h | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/module.lds.h b/arch/arm64/include/asm/module.lds.h
>> index a11ccadd47d2..094701ec5500 100644
>> --- a/arch/arm64/include/asm/module.lds.h
>> +++ b/arch/arm64/include/asm/module.lds.h
>> @@ -1,8 +1,8 @@
>> SECTIONS {
>> #ifdef CONFIG_ARM64_MODULE_PLTS
>> - .plt 0 (NOLOAD) : { BYTE(0) }
>> - .init.plt 0 (NOLOAD) : { BYTE(0) }
>> - .text.ftrace_trampoline 0 (NOLOAD) : { BYTE(0) }
>> + .plt 0 : { BYTE(0) }
>> + .init.plt 0 : { BYTE(0) }
>> + .text.ftrace_trampoline 0 : { BYTE(0) }
>> #endif
>>
>> #ifdef CONFIG_KASAN_SW_TAGS
>> --
>> 2.35.1.265.g69c8d7142f-goog
>>

2022-02-18 19:46:46

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH] arm64 module: remove (NOLOAD)

On Fri, 18 Feb 2022 at 17:34, Nathan Chancellor <[email protected]> wrote:
>
> On Fri, Feb 18, 2022 at 12:50:16AM -0800, Fangrui Song wrote:
> > On 2022-02-18, Ard Biesheuvel wrote:
> > > On Fri, 18 Feb 2022 at 09:12, Fangrui Song <[email protected]> wrote:
> > > >
> > > > On ELF, (NOLOAD) sets the section type to SHT_NOBITS[1]. It is conceptually
> > > > inappropriate for .plt and .text.* sections which are always
> > > > SHT_PROGBITS.
> > > >
> > > > In GNU ld, if PLT entries are needed, .plt will be SHT_PROGBITS anyway
> > > > and (NOLOAD) will be essentially ignored. In ld.lld, since
> > > > https://reviews.llvm.org/D118840 ("[ELF] Support (TYPE=<value>) to
> > > > customize the output section type"), ld.lld will report a `section type
> > > > mismatch` error. Just remove (NOLOAD) to fix the error.
> > > >
> > > > [1] https://lld.llvm.org/ELF/linker_script.html As of today, "The
> > > > section should be marked as not loadable" on
> > > > https://sourceware.org/binutils/docs/ld/Output-Section-Type.html is
> > > > outdated for ELF.
> > >
> > > This patch lacks a SOB line.
> > >
> > > With one added,
> > >
> > > Acked-by: Ard Biesheuvel <[email protected]>
> >
> > Ah, yes. Sorry, I haven't sent a kernel patch for a while...
> >
> > Reported-by: Nathan Chancellor <[email protected]>
> > Signed-off-by: Fangrui Song <[email protected]>
> > Acked-by: Ard Biesheuvel <[email protected]>
>
> I am assuming this patch is the solution we want, as opposed to Ard's
> suggestion of renaming these sections at
> https://reviews.llvm.org/D118840 (unless that was a tangential
> suggestion).
>

Renaming will not make the problem go away. It will only clear up the
confusion regarding the contents of these sections, which are not in
fact linker emitted PLT entries, but branching veneers emitted by the
kernel's module loader.

> I have verified that modules still load. Additionally, this needs to go
> to stable so that groups who upgrade their toolchain to a revision that
> include the LLD patch don't get broken as well.
>
> Cc: [email protected]
> Tested-by: Nathan Chancellor <[email protected]>
>
>
> > > > ---
> > > > arch/arm64/include/asm/module.lds.h | 6 +++---
> > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/include/asm/module.lds.h b/arch/arm64/include/asm/module.lds.h
> > > > index a11ccadd47d2..094701ec5500 100644
> > > > --- a/arch/arm64/include/asm/module.lds.h
> > > > +++ b/arch/arm64/include/asm/module.lds.h
> > > > @@ -1,8 +1,8 @@
> > > > SECTIONS {
> > > > #ifdef CONFIG_ARM64_MODULE_PLTS
> > > > - .plt 0 (NOLOAD) : { BYTE(0) }
> > > > - .init.plt 0 (NOLOAD) : { BYTE(0) }
> > > > - .text.ftrace_trampoline 0 (NOLOAD) : { BYTE(0) }
> > > > + .plt 0 : { BYTE(0) }
> > > > + .init.plt 0 : { BYTE(0) }
> > > > + .text.ftrace_trampoline 0 : { BYTE(0) }
> > > > #endif
> > > >
> > > > #ifdef CONFIG_KASAN_SW_TAGS
> > > > --
> > > > 2.35.1.265.g69c8d7142f-goog
> > > >
> >

2022-02-19 00:57:07

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] arm64 module: remove (NOLOAD)

On Fri, Feb 18, 2022 at 12:50:16AM -0800, Fangrui Song wrote:
> On 2022-02-18, Ard Biesheuvel wrote:
> > On Fri, 18 Feb 2022 at 09:12, Fangrui Song <[email protected]> wrote:
> > >
> > > On ELF, (NOLOAD) sets the section type to SHT_NOBITS[1]. It is conceptually
> > > inappropriate for .plt and .text.* sections which are always
> > > SHT_PROGBITS.
> > >
> > > In GNU ld, if PLT entries are needed, .plt will be SHT_PROGBITS anyway
> > > and (NOLOAD) will be essentially ignored. In ld.lld, since
> > > https://reviews.llvm.org/D118840 ("[ELF] Support (TYPE=<value>) to
> > > customize the output section type"), ld.lld will report a `section type
> > > mismatch` error. Just remove (NOLOAD) to fix the error.
> > >
> > > [1] https://lld.llvm.org/ELF/linker_script.html As of today, "The
> > > section should be marked as not loadable" on
> > > https://sourceware.org/binutils/docs/ld/Output-Section-Type.html is
> > > outdated for ELF.
> >
> > This patch lacks a SOB line.
> >
> > With one added,
> >
> > Acked-by: Ard Biesheuvel <[email protected]>
>
> Ah, yes. Sorry, I haven't sent a kernel patch for a while...
>
> Reported-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Fangrui Song <[email protected]>
> Acked-by: Ard Biesheuvel <[email protected]>

I am assuming this patch is the solution we want, as opposed to Ard's
suggestion of renaming these sections at
https://reviews.llvm.org/D118840 (unless that was a tangential
suggestion).

I have verified that modules still load. Additionally, this needs to go
to stable so that groups who upgrade their toolchain to a revision that
include the LLD patch don't get broken as well.

Cc: [email protected]
Tested-by: Nathan Chancellor <[email protected]>


> > > ---
> > > arch/arm64/include/asm/module.lds.h | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/module.lds.h b/arch/arm64/include/asm/module.lds.h
> > > index a11ccadd47d2..094701ec5500 100644
> > > --- a/arch/arm64/include/asm/module.lds.h
> > > +++ b/arch/arm64/include/asm/module.lds.h
> > > @@ -1,8 +1,8 @@
> > > SECTIONS {
> > > #ifdef CONFIG_ARM64_MODULE_PLTS
> > > - .plt 0 (NOLOAD) : { BYTE(0) }
> > > - .init.plt 0 (NOLOAD) : { BYTE(0) }
> > > - .text.ftrace_trampoline 0 (NOLOAD) : { BYTE(0) }
> > > + .plt 0 : { BYTE(0) }
> > > + .init.plt 0 : { BYTE(0) }
> > > + .text.ftrace_trampoline 0 : { BYTE(0) }
> > > #endif
> > >
> > > #ifdef CONFIG_KASAN_SW_TAGS
> > > --
> > > 2.35.1.265.g69c8d7142f-goog
> > >
>

2022-02-25 21:16:45

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH] arm64 module: remove (NOLOAD)

On Fri, 18 Feb 2022 00:12:09 -0800, Fangrui Song wrote:
> On ELF, (NOLOAD) sets the section type to SHT_NOBITS[1]. It is conceptually
> inappropriate for .plt and .text.* sections which are always
> SHT_PROGBITS.
>
> In GNU ld, if PLT entries are needed, .plt will be SHT_PROGBITS anyway
> and (NOLOAD) will be essentially ignored. In ld.lld, since
> https://reviews.llvm.org/D118840 ("[ELF] Support (TYPE=<value>) to
> customize the output section type"), ld.lld will report a `section type
> mismatch` error. Just remove (NOLOAD) to fix the error.
>
> [...]

Applied to arm64 (for-next/linkage), thanks!

[1/1] arm64: module: remove (NOLOAD) from linker script
https://git.kernel.org/arm64/c/4013e26670c5

Cheers,
--
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

2022-03-09 08:02:35

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: x86/core] arm64: module: remove (NOLOAD) from linker script

The following commit has been merged into the x86/core branch of tip:

Commit-ID: 4013e26670c590944abdab56c4fa797527b74325
Gitweb: https://git.kernel.org/tip/4013e26670c590944abdab56c4fa797527b74325
Author: Fangrui Song <[email protected]>
AuthorDate: Fri, 18 Feb 2022 00:12:09 -08:00
Committer: Will Deacon <[email protected]>
CommitterDate: Fri, 25 Feb 2022 14:06:50

arm64: module: remove (NOLOAD) from linker script

On ELF, (NOLOAD) sets the section type to SHT_NOBITS[1]. It is conceptually
inappropriate for .plt and .text.* sections which are always
SHT_PROGBITS.

In GNU ld, if PLT entries are needed, .plt will be SHT_PROGBITS anyway
and (NOLOAD) will be essentially ignored. In ld.lld, since
https://reviews.llvm.org/D118840 ("[ELF] Support (TYPE=<value>) to
customize the output section type"), ld.lld will report a `section type
mismatch` error. Just remove (NOLOAD) to fix the error.

[1] https://lld.llvm.org/ELF/linker_script.html As of today, "The
section should be marked as not loadable" on
https://sourceware.org/binutils/docs/ld/Output-Section-Type.html is
outdated for ELF.

Tested-by: Nathan Chancellor <[email protected]>
Reported-by: Nathan Chancellor <[email protected]>
Signed-off-by: Fangrui Song <[email protected]>
Acked-by: Ard Biesheuvel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Will Deacon <[email protected]>
---
arch/arm64/include/asm/module.lds.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/module.lds.h b/arch/arm64/include/asm/module.lds.h
index a11ccad..094701e 100644
--- a/arch/arm64/include/asm/module.lds.h
+++ b/arch/arm64/include/asm/module.lds.h
@@ -1,8 +1,8 @@
SECTIONS {
#ifdef CONFIG_ARM64_MODULE_PLTS
- .plt 0 (NOLOAD) : { BYTE(0) }
- .init.plt 0 (NOLOAD) : { BYTE(0) }
- .text.ftrace_trampoline 0 (NOLOAD) : { BYTE(0) }
+ .plt 0 : { BYTE(0) }
+ .init.plt 0 : { BYTE(0) }
+ .text.ftrace_trampoline 0 : { BYTE(0) }
#endif

#ifdef CONFIG_KASAN_SW_TAGS