| Message ID | 20220218081209.354383-1-maskray@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64 module: remove (NOLOAD) | expand |
On Fri, 18 Feb 2022 at 09:12, Fangrui Song <maskray@google.com> 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 <ardb@kernel.org> > --- > 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 >
On 2022-02-18, Ard Biesheuvel wrote: >On Fri, 18 Feb 2022 at 09:12, Fangrui Song <maskray@google.com> 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 <ardb@kernel.org> Ah, yes. Sorry, I haven't sent a kernel patch for a while... Reported-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Fangrui Song <maskray@google.com> Acked-by: Ard Biesheuvel <ardb@kernel.org> >> --- >> 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 >>
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 <maskray@google.com> 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 <ardb@kernel.org> > > Ah, yes. Sorry, I haven't sent a kernel patch for a while... > > Reported-by: Nathan Chancellor <nathan@kernel.org> > Signed-off-by: Fangrui Song <maskray@google.com> > Acked-by: Ard Biesheuvel <ardb@kernel.org> 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: stable@vger.kernel.org Tested-by: Nathan Chancellor <nathan@kernel.org> > > > --- > > > 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 > > > >
On Fri, 18 Feb 2022 at 17:34, Nathan Chancellor <nathan@kernel.org> 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 <maskray@google.com> 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 <ardb@kernel.org> > > > > Ah, yes. Sorry, I haven't sent a kernel patch for a while... > > > > Reported-by: Nathan Chancellor <nathan@kernel.org> > > Signed-off-by: Fangrui Song <maskray@google.com> > > Acked-by: Ard Biesheuvel <ardb@kernel.org> > > 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: stable@vger.kernel.org > Tested-by: Nathan Chancellor <nathan@kernel.org> > > > > > > --- > > > > 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 > > > > > >
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,
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
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(-)