public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] arm64 module: remove (NOLOAD)
@ 2022-02-18  8:12 Fangrui Song
  2022-02-18  8:17 ` Ard Biesheuvel
  2022-02-25 15:50 ` Will Deacon
  0 siblings, 2 replies; 6+ messages in thread
From: Fangrui Song @ 2022-02-18  8:12 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Ard Biesheuvel, linux-arm-kernel
  Cc: Nathan Chancellor, linux-kernel, llvm, Fangrui Song

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] arm64 module: remove (NOLOAD)
  2022-02-18  8:12 [PATCH] arm64 module: remove (NOLOAD) Fangrui Song
@ 2022-02-18  8:17 ` Ard Biesheuvel
  2022-02-18  8:50   ` Fangrui Song
  2022-02-25 15:50 ` Will Deacon
  1 sibling, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2022-02-18  8:17 UTC (permalink / raw)
  To: Fangrui Song
  Cc: Catalin Marinas, Will Deacon, Linux ARM, Nathan Chancellor,
	Linux Kernel Mailing List, llvm

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
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] arm64 module: remove (NOLOAD)
  2022-02-18  8:17 ` Ard Biesheuvel
@ 2022-02-18  8:50   ` Fangrui Song
  2022-02-18 16:34     ` Nathan Chancellor
  0 siblings, 1 reply; 6+ messages in thread
From: Fangrui Song @ 2022-02-18  8:50 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Catalin Marinas, Will Deacon, Linux ARM, Nathan Chancellor,
	Linux Kernel Mailing List, llvm

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
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] arm64 module: remove (NOLOAD)
  2022-02-18  8:50   ` Fangrui Song
@ 2022-02-18 16:34     ` Nathan Chancellor
  2022-02-18 19:08       ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Nathan Chancellor @ 2022-02-18 16:34 UTC (permalink / raw)
  To: Fangrui Song
  Cc: Ard Biesheuvel, Catalin Marinas, Will Deacon, Linux ARM,
	Linux Kernel Mailing List, llvm

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
> > > 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] arm64 module: remove (NOLOAD)
  2022-02-18 16:34     ` Nathan Chancellor
@ 2022-02-18 19:08       ` Ard Biesheuvel
  0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2022-02-18 19:08 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Fangrui Song, Catalin Marinas, Will Deacon, Linux ARM,
	Linux Kernel Mailing List, llvm

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
> > > >
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] arm64 module: remove (NOLOAD)
  2022-02-18  8:12 [PATCH] arm64 module: remove (NOLOAD) Fangrui Song
  2022-02-18  8:17 ` Ard Biesheuvel
@ 2022-02-25 15:50 ` Will Deacon
  1 sibling, 0 replies; 6+ messages in thread
From: Will Deacon @ 2022-02-25 15:50 UTC (permalink / raw)
  To: Fangrui Song, linux-arm-kernel, Ard Biesheuvel, Catalin Marinas
  Cc: kernel-team, Will Deacon, linux-kernel, Nathan Chancellor, llvm

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-25 15:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-18  8:12 [PATCH] arm64 module: remove (NOLOAD) Fangrui Song
2022-02-18  8:17 ` Ard Biesheuvel
2022-02-18  8:50   ` Fangrui Song
2022-02-18 16:34     ` Nathan Chancellor
2022-02-18 19:08       ` Ard Biesheuvel
2022-02-25 15:50 ` Will Deacon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox