This series hunts the problems discovered after manual enabling of
ARCH_WANT_LD_ORPHAN_WARN, notably the missing PAGE_ALIGNED_DATA()
section affecting VDSO placement (marked for stable).
Compile and runtime tested on MIPS32R2 CPS board with no issues.
Since v1 [0]:
- catch .got entries too as LLD may produce it (Nathan);
- check for unwanted sections to be zero-sized instead of
discarding (Fangrui).
[0] https://lore.kernel.org/linux-mips/[email protected]
Alexander Lobakin (4):
MIPS: vmlinux.lds.S: add missing PAGE_ALIGNED_DATA() section
MIPS: vmlinux.lds.S: add ".gnu.attributes" to DISCARDS
MIPS: vmlinux.lds.S: catch bad .got, .plt and .rel.dyn at link time
MIPS: select ARCH_WANT_LD_ORPHAN_WARN
arch/mips/Kconfig | 1 +
arch/mips/kernel/vmlinux.lds.S | 39 +++++++++++++++++++++++++++++++++-
2 files changed, 39 insertions(+), 1 deletion(-)
--
2.30.0
MIPS uses its own declaration of rwdata, and thus it should be kept
in sync with the asm-generic one. Currently PAGE_ALIGNED_DATA() is
missing from the linker script, which emits the following ld
warnings:
mips-alpine-linux-musl-ld: warning: orphan section
`.data..page_aligned' from `arch/mips/kernel/vdso.o' being placed
in section `.data..page_aligned'
mips-alpine-linux-musl-ld: warning: orphan section
`.data..page_aligned' from `arch/mips/vdso/vdso-image.o' being placed
in section `.data..page_aligned'
Add the necessary declaration, so the mentioned structures will be
placed in vmlinux as intended:
ffffffff80630580 D __end_once
ffffffff80630580 D __start___dyndbg
ffffffff80630580 D __start_once
ffffffff80630580 D __stop___dyndbg
ffffffff80634000 d mips_vdso_data
ffffffff80638000 d vdso_data
ffffffff80638580 D _gp
ffffffff8063c000 T __init_begin
ffffffff8063c000 D _edata
ffffffff8063c000 T _sinittext
->
ffffffff805a4000 D __end_init_task
ffffffff805a4000 D __nosave_begin
ffffffff805a4000 D __nosave_end
ffffffff805a4000 d mips_vdso_data
ffffffff805a8000 d vdso_data
ffffffff805ac000 D mmlist_lock
ffffffff805ac080 D tasklist_lock
Fixes: ebb5e78cc634 ("MIPS: Initial implementation of a VDSO")
Cc: [email protected] # 4.4+
Signed-off-by: Alexander Lobakin <[email protected]>
---
arch/mips/kernel/vmlinux.lds.S | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
index 5e97e9d02f98..83e27a181206 100644
--- a/arch/mips/kernel/vmlinux.lds.S
+++ b/arch/mips/kernel/vmlinux.lds.S
@@ -90,6 +90,7 @@ SECTIONS
INIT_TASK_DATA(THREAD_SIZE)
NOSAVE_DATA
+ PAGE_ALIGNED_DATA(PAGE_SIZE)
CACHELINE_ALIGNED_DATA(1 << CONFIG_MIPS_L1_CACHE_SHIFT)
READ_MOSTLY_DATA(1 << CONFIG_MIPS_L1_CACHE_SHIFT)
DATA_DATA
--
2.30.0
Catch any symbols placed in .got, .got.plt, .plt, .rel.dyn
or .rela.dyn and check for these sections to be zero-sized
at link time.
At least two of them were noticed in real builds:
mips-alpine-linux-musl-ld: warning: orphan section `.rel.dyn'
from `init/main.o' being placed in section `.rel.dyn'
ld.lld: warning: <internal>:(.got) is being placed in '.got'
Adopted from x86/kernel/vmlinux.lds.S.
Reported-by: Nathan Chancellor <[email protected]> # .got
Suggested-by: Fangrui Song <[email protected]> # .rel.dyn
Signed-off-by: Alexander Lobakin <[email protected]>
---
arch/mips/kernel/vmlinux.lds.S | 35 ++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
index 5d6563970ab2..05eda9d9a7d5 100644
--- a/arch/mips/kernel/vmlinux.lds.S
+++ b/arch/mips/kernel/vmlinux.lds.S
@@ -227,4 +227,39 @@ SECTIONS
*(.pdr)
*(.reginfo)
}
+
+ /*
+ * Sections that should stay zero sized, which is safer to
+ * explicitly check instead of blindly discarding.
+ */
+
+ .got : {
+ *(.got)
+ *(.igot.*)
+ }
+ ASSERT(SIZEOF(.got) == 0, "Unexpected GOT entries detected!")
+
+ .got.plt (INFO) : {
+ *(.got.plt)
+ }
+ ASSERT(SIZEOF(.got.plt) == 0, "Unexpected GOT/PLT entries detected!")
+
+ .plt : {
+ *(.plt)
+ *(.plt.*)
+ *(.iplt)
+ }
+ ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure linkages detected!")
+
+ .rel.dyn : {
+ *(.rel.*)
+ *(.rel_*)
+ }
+ ASSERT(SIZEOF(.rel.dyn) == 0, "Unexpected run-time relocations (.rel) detected!")
+
+ .rela.dyn : {
+ *(.rela.*)
+ *(.rela_*)
+ }
+ ASSERT(SIZEOF(.rela.dyn) == 0, "Unexpected run-time relocations (.rela) detected!")
}
--
2.30.0
Now, after that all the sections are explicitly described and
declared in vmlinux.lds.S, we can enable ld orphan warnings to
prevent from missing any new sections in future.
Signed-off-by: Alexander Lobakin <[email protected]>
---
arch/mips/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index d68df1febd25..d3e64cc0932b 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -18,6 +18,7 @@ config MIPS
select ARCH_USE_QUEUED_SPINLOCKS
select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU
select ARCH_WANT_IPC_PARSE_VERSION
+ select ARCH_WANT_LD_ORPHAN_WARN
select BUILDTIME_TABLE_SORT
select CLONE_BACKWARDS
select CPU_NO_EFFICIENT_FFS if (TARGET_ISA_REV < 1)
--
2.30.0
On Wed, Jan 6, 2021 at 12:08 PM Alexander Lobakin <[email protected]> wrote:
>
> Catch any symbols placed in .got, .got.plt, .plt, .rel.dyn
> or .rela.dyn and check for these sections to be zero-sized
> at link time.
>
> At least two of them were noticed in real builds:
>
> mips-alpine-linux-musl-ld: warning: orphan section `.rel.dyn'
> from `init/main.o' being placed in section `.rel.dyn'
>
> ld.lld: warning: <internal>:(.got) is being placed in '.got'
>
> Adopted from x86/kernel/vmlinux.lds.S.
>
> Reported-by: Nathan Chancellor <[email protected]> # .got
> Suggested-by: Fangrui Song <[email protected]> # .rel.dyn
> Signed-off-by: Alexander Lobakin <[email protected]>
> ---
> arch/mips/kernel/vmlinux.lds.S | 35 ++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
> index 5d6563970ab2..05eda9d9a7d5 100644
> --- a/arch/mips/kernel/vmlinux.lds.S
> +++ b/arch/mips/kernel/vmlinux.lds.S
> @@ -227,4 +227,39 @@ SECTIONS
> *(.pdr)
> *(.reginfo)
> }
> +
> + /*
> + * Sections that should stay zero sized, which is safer to
> + * explicitly check instead of blindly discarding.
> + */
> +
> + .got : {
> + *(.got)
> + *(.igot.*)
> + }
> + ASSERT(SIZEOF(.got) == 0, "Unexpected GOT entries detected!")
> +
> + .got.plt (INFO) : {
> + *(.got.plt)
> + }
> + ASSERT(SIZEOF(.got.plt) == 0, "Unexpected GOT/PLT entries detected!")
(INFO) drops the SHF_ALLOC flag from the output section (It does not
mean "informational"). INFO is not need here.
The diff from 815d680771ae09080d2da83dac2647c08cdf99ce "x86/build:
Enforce an empty .got.plt section" is not needed.
> + .plt : {
> + *(.plt)
> + *(.plt.*)
> + *(.iplt)
> + }
> + ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure linkages detected!")
> +
> + .rel.dyn : {
> + *(.rel.*)
> + *(.rel_*)
> + }
> + ASSERT(SIZEOF(.rel.dyn) == 0, "Unexpected run-time relocations (.rel) detected!")
> +
> + .rela.dyn : {
> + *(.rela.*)
> + *(.rela_*)
> + }
> + ASSERT(SIZEOF(.rela.dyn) == 0, "Unexpected run-time relocations (.rela) detected!")
> }
x86 has both .rel.dyn and .rela.dyn because i386 psABI uses REL while
x86-64 psABI uses RELA, but mips does not need .rela.dyn
> --
> 2.30.0
>
>
On Wed, Jan 06, 2021 at 08:08:29PM +0000, Alexander Lobakin wrote:
> Catch any symbols placed in .got, .got.plt, .plt, .rel.dyn
> or .rela.dyn and check for these sections to be zero-sized
> at link time.
>
> At least two of them were noticed in real builds:
>
> mips-alpine-linux-musl-ld: warning: orphan section `.rel.dyn'
> from `init/main.o' being placed in section `.rel.dyn'
>
> ld.lld: warning: <internal>:(.got) is being placed in '.got'
>
> Adopted from x86/kernel/vmlinux.lds.S.
>
> Reported-by: Nathan Chancellor <[email protected]> # .got
> Suggested-by: Fangrui Song <[email protected]> # .rel.dyn
> Signed-off-by: Alexander Lobakin <[email protected]>
> ---
> arch/mips/kernel/vmlinux.lds.S | 35 ++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
> index 5d6563970ab2..05eda9d9a7d5 100644
> --- a/arch/mips/kernel/vmlinux.lds.S
> +++ b/arch/mips/kernel/vmlinux.lds.S
> @@ -227,4 +227,39 @@ SECTIONS
> *(.pdr)
> *(.reginfo)
> }
> +
> + /*
> + * Sections that should stay zero sized, which is safer to
> + * explicitly check instead of blindly discarding.
> + */
> +
> + .got : {
> + *(.got)
> + *(.igot.*)
> + }
> + ASSERT(SIZEOF(.got) == 0, "Unexpected GOT entries detected!")
This assertion does trigger now.
$ make -skj"$(nproc)" ARCH=mips CROSS_COMPILE=mipsel-linux-gnu- LLVM=1 \
O=out/mipsel distclean malta_kvm_guest_defconfig all
...
ld.lld: error: Unexpected GOT entries detected!
ld.lld: error: Unexpected GOT entries detected!
...
> + .got.plt (INFO) : {
> + *(.got.plt)
> + }
> + ASSERT(SIZEOF(.got.plt) == 0, "Unexpected GOT/PLT entries detected!")
> +
> + .plt : {
> + *(.plt)
> + *(.plt.*)
> + *(.iplt)
> + }
> + ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure linkages detected!")
> +
> + .rel.dyn : {
> + *(.rel.*)
> + *(.rel_*)
> + }
> + ASSERT(SIZEOF(.rel.dyn) == 0, "Unexpected run-time relocations (.rel) detected!")
> +
> + .rela.dyn : {
> + *(.rela.*)
> + *(.rela_*)
> + }
> + ASSERT(SIZEOF(.rela.dyn) == 0, "Unexpected run-time relocations (.rela) detected!")
> }
> --
> 2.30.0
>
>
From: Nathan Chancellor <[email protected]>
Date: Wed, 6 Jan 2021 13:23:24 -0700
On Wed, Jan 06, 2021 at 08:08:29PM +0000, Alexander Lobakin wrote:
>> Catch any symbols placed in .got, .got.plt, .plt, .rel.dyn
>> or .rela.dyn and check for these sections to be zero-sized
>> at link time.
>>
>> At least two of them were noticed in real builds:
>>
>> mips-alpine-linux-musl-ld: warning: orphan section `.rel.dyn'
>> from `init/main.o' being placed in section `.rel.dyn'
>>
>> ld.lld: warning: <internal>:(.got) is being placed in '.got'
>>
>> Adopted from x86/kernel/vmlinux.lds.S.
>>
>> Reported-by: Nathan Chancellor <[email protected]> # .got
>> Suggested-by: Fangrui Song <[email protected]> # .rel.dyn
>> Signed-off-by: Alexander Lobakin <[email protected]>
>> ---
>> arch/mips/kernel/vmlinux.lds.S | 35 ++++++++++++++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>>
>> diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
>> index 5d6563970ab2..05eda9d9a7d5 100644
>> --- a/arch/mips/kernel/vmlinux.lds.S
>> +++ b/arch/mips/kernel/vmlinux.lds.S
>> @@ -227,4 +227,39 @@ SECTIONS
>> *(.pdr)
>> *(.reginfo)
>> }
>> +
>> + /*
>> + * Sections that should stay zero sized, which is safer to
>> + * explicitly check instead of blindly discarding.
>> + */
>> +
>> + .got : {
>> + *(.got)
>> + *(.igot.*)
>> + }
>> + ASSERT(SIZEOF(.got) == 0, "Unexpected GOT entries detected!")
>
> This assertion does trigger now.
>
> $ make -skj"$(nproc)" ARCH=mips CROSS_COMPILE=mipsel-linux-gnu- LLVM=1 \
> O=out/mipsel distclean malta_kvm_guest_defconfig all
> ...
> ld.lld: error: Unexpected GOT entries detected!
> ld.lld: error: Unexpected GOT entries detected!
> ...
Oops. I'll build my kernel with LLVM stack and dig into it deeper
tomorrow.
>> + .got.plt (INFO) : {
>> + *(.got.plt)
>> + }
>> + ASSERT(SIZEOF(.got.plt) == 0, "Unexpected GOT/PLT entries detected!")
>> +
>> + .plt : {
>> + *(.plt)
>> + *(.plt.*)
>> + *(.iplt)
>> + }
>> + ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure linkages detected!")
>> +
>> + .rel.dyn : {
>> + *(.rel.*)
>> + *(.rel_*)
>> + }
>> + ASSERT(SIZEOF(.rel.dyn) == 0, "Unexpected run-time relocations (.rel) detected!")
>> +
>> + .rela.dyn : {
>> + *(.rela.*)
>> + *(.rela_*)
>> + }
>> + ASSERT(SIZEOF(.rela.dyn) == 0, "Unexpected run-time relocations (.rela) detected!")
>> }
>> --
>> 2.30.0
Thanks,
Al
On Wed, Jan 06, 2021 at 08:08:14PM +0000, Alexander Lobakin wrote:
> MIPS uses its own declaration of rwdata, and thus it should be kept
> in sync with the asm-generic one. Currently PAGE_ALIGNED_DATA() is
> missing from the linker script, which emits the following ld
> warnings:
>
> mips-alpine-linux-musl-ld: warning: orphan section
> `.data..page_aligned' from `arch/mips/kernel/vdso.o' being placed
> in section `.data..page_aligned'
> mips-alpine-linux-musl-ld: warning: orphan section
> `.data..page_aligned' from `arch/mips/vdso/vdso-image.o' being placed
> in section `.data..page_aligned'
>
> Add the necessary declaration, so the mentioned structures will be
> placed in vmlinux as intended:
>
> ffffffff80630580 D __end_once
> ffffffff80630580 D __start___dyndbg
> ffffffff80630580 D __start_once
> ffffffff80630580 D __stop___dyndbg
> ffffffff80634000 d mips_vdso_data
> ffffffff80638000 d vdso_data
> ffffffff80638580 D _gp
> ffffffff8063c000 T __init_begin
> ffffffff8063c000 D _edata
> ffffffff8063c000 T _sinittext
>
> ->
>
> ffffffff805a4000 D __end_init_task
> ffffffff805a4000 D __nosave_begin
> ffffffff805a4000 D __nosave_end
> ffffffff805a4000 d mips_vdso_data
> ffffffff805a8000 d vdso_data
> ffffffff805ac000 D mmlist_lock
> ffffffff805ac080 D tasklist_lock
>
> Fixes: ebb5e78cc634 ("MIPS: Initial implementation of a VDSO")
> Cc: [email protected] # 4.4+
> Signed-off-by: Alexander Lobakin <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Wed, Jan 06, 2021 at 08:08:46PM +0000, Alexander Lobakin wrote:
> Now, after that all the sections are explicitly described and
> declared in vmlinux.lds.S, we can enable ld orphan warnings to
> prevent from missing any new sections in future.
>
> Signed-off-by: Alexander Lobakin <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook