* Re: [PATCH v4] MIPS: Truncate link address into 32bit for 32bit kernel
2020-04-13 6:26 [PATCH v4] MIPS: Truncate link address into 32bit for 32bit kernel Jiaxun Yang
@ 2020-04-13 6:59 ` Maciej W. Rozycki
2020-04-13 7:32 ` Jiaxun Yang
2020-04-13 16:25 ` Kees Cook
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Maciej W. Rozycki @ 2020-04-13 6:59 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-mips, clang-built-linux, Fangrui Song, Nathan Chancellor,
Thomas Bogendoerfer, Borislav Petkov, Kees Cook, Heiko Carstens,
linux-kernel
On Mon, 13 Apr 2020, Jiaxun Yang wrote:
> LLD failed to link vmlinux with 64bit load address for 32bit ELF
> while bfd will strip 64bit address into 32bit silently.
> To fix LLD build, we should truncate load address provided by platform
> into 32bit for 32bit kernel.
Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>
> diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
> index a5f00ec73ea6..5226cd8e4bee 100644
> --- a/arch/mips/kernel/vmlinux.lds.S
> +++ b/arch/mips/kernel/vmlinux.lds.S
> @@ -55,7 +55,7 @@ SECTIONS
> /* . = 0xa800000000300000; */
> . = 0xffffffff80300000;
> #endif
> - . = VMLINUX_LOAD_ADDRESS;
> + . = VMLINUX_LINK_ADDRESS;
The CONFIG_BOOT_ELF64 cruft right above it looks interesting to me, never
have ever been used. We have had the current arrangement since:
commit 923ec3d20eef9e36456868b590873ce39f17fe71
Author: Ralf Baechle <ralf@linux-mips.org>
Date: Wed Nov 6 22:16:38 2002 +0000
Define load address in linker script instead of relying on the
deprecated and notoriously unreliable option -Ttext.
and previously `-Ttext' was used with this script anyway, though not very
long, as the script was entirely ignored until:
commit 7a782968041ffc4c2d89816238e2f8ea5cceddba
Author: Ralf Baechle <ralf@linux-mips.org>
Date: Thu Oct 31 23:54:21 2002 +0000
Merge with Linux 2.5.36.
Maciej
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v4] MIPS: Truncate link address into 32bit for 32bit kernel
2020-04-13 6:59 ` Maciej W. Rozycki
@ 2020-04-13 7:32 ` Jiaxun Yang
2020-04-13 15:34 ` Fangrui Song
2020-04-13 20:06 ` Maciej W. Rozycki
0 siblings, 2 replies; 16+ messages in thread
From: Jiaxun Yang @ 2020-04-13 7:32 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: linux-mips, clang-built-linux, Fangrui Song, Nathan Chancellor,
Thomas Bogendoerfer, Borislav Petkov, Kees Cook, Heiko Carstens,
linux-kernel
On Mon, 13 Apr 2020 07:59:29 +0100 (BST)
"Maciej W. Rozycki" <macro@linux-mips.org> wrote:
> On Mon, 13 Apr 2020, Jiaxun Yang wrote:
>
> > LLD failed to link vmlinux with 64bit load address for 32bit ELF
> > while bfd will strip 64bit address into 32bit silently.
> > To fix LLD build, we should truncate load address provided by
> > platform into 32bit for 32bit kernel.
>
> Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>
>
> > diff --git a/arch/mips/kernel/vmlinux.lds.S
> > b/arch/mips/kernel/vmlinux.lds.S index a5f00ec73ea6..5226cd8e4bee
> > 100644 --- a/arch/mips/kernel/vmlinux.lds.S
> > +++ b/arch/mips/kernel/vmlinux.lds.S
> > @@ -55,7 +55,7 @@ SECTIONS
> > /* . = 0xa800000000300000; */
> > . = 0xffffffff80300000;
> > #endif
> > - . = VMLINUX_LOAD_ADDRESS;
> > + . = VMLINUX_LINK_ADDRESS;
>
> The CONFIG_BOOT_ELF64 cruft right above it looks interesting to me,
> never have ever been used. We have had the current arrangement since:
It confused me either.
It's only used by SGI so probably it's time to rename it as
BOOT_SEG_ELF64.
Wish someone could clarify what is it.
Thanks.
>
> commit 923ec3d20eef9e36456868b590873ce39f17fe71
> Author: Ralf Baechle <ralf@linux-mips.org>
> Date: Wed Nov 6 22:16:38 2002 +0000
>
> Define load address in linker script instead of relying on the
> deprecated and notoriously unreliable option -Ttext.
>
> and previously `-Ttext' was used with this script anyway, though not
> very long, as the script was entirely ignored until:
>
> commit 7a782968041ffc4c2d89816238e2f8ea5cceddba
> Author: Ralf Baechle <ralf@linux-mips.org>
> Date: Thu Oct 31 23:54:21 2002 +0000
>
> Merge with Linux 2.5.36.
>
> Maciej
--
Jiaxun Yang
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] MIPS: Truncate link address into 32bit for 32bit kernel
2020-04-13 7:32 ` Jiaxun Yang
@ 2020-04-13 15:34 ` Fangrui Song
2020-04-13 20:08 ` Maciej W. Rozycki
2020-04-13 20:06 ` Maciej W. Rozycki
1 sibling, 1 reply; 16+ messages in thread
From: Fangrui Song @ 2020-04-13 15:34 UTC (permalink / raw)
To: Jiaxun Yang
Cc: Maciej W. Rozycki, linux-mips, clang-built-linux,
Nathan Chancellor, Thomas Bogendoerfer, Borislav Petkov,
Kees Cook, Heiko Carstens, linux-kernel
On 2020-04-13, Jiaxun Yang wrote:
>On Mon, 13 Apr 2020 07:59:29 +0100 (BST)
>"Maciej W. Rozycki" <macro@linux-mips.org> wrote:
>
>> On Mon, 13 Apr 2020, Jiaxun Yang wrote:
>>
>> > LLD failed to link vmlinux with 64bit load address for 32bit ELF
>> > while bfd will strip 64bit address into 32bit silently.
>> > To fix LLD build, we should truncate load address provided by
>> > platform into 32bit for 32bit kernel.
>>
>> Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>
>>
>> > diff --git a/arch/mips/kernel/vmlinux.lds.S
>> > b/arch/mips/kernel/vmlinux.lds.S index a5f00ec73ea6..5226cd8e4bee
>> > 100644 --- a/arch/mips/kernel/vmlinux.lds.S
>> > +++ b/arch/mips/kernel/vmlinux.lds.S
>> > @@ -55,7 +55,7 @@ SECTIONS
>> > /* . = 0xa800000000300000; */
>> > . = 0xffffffff80300000;
>> > #endif
>> > - . = VMLINUX_LOAD_ADDRESS;
>> > + . = VMLINUX_LINK_ADDRESS;
>>
>> The CONFIG_BOOT_ELF64 cruft right above it looks interesting to me,
>> never have ever been used. We have had the current arrangement since:
>
>It confused me either.
>It's only used by SGI so probably it's time to rename it as
>BOOT_SEG_ELF64.
>
>Wish someone could clarify what is it.
>
>Thanks.
Agreed that -Ttext in
arch/mips/boot/compressed/Makefile
100: cmd_zld = $(LD) $(KBUILD_LDFLAGS) -Ttext $(VMLINUZ_LOAD_ADDRESS) -T $< $(vmlinuzobjs-y) -o $@
and a few other places are brittle. They need to be replaced with Output Section Address:
(https://sourceware.org/binutils/docs/ld/Output-Section-Address.html
https://github.com/llvm/llvm-project/blob/master/lld/docs/ELF/linker_script.rst#output-section-address)
-Ttext changes the address of .text . This can lead to the change of the
address of the text segment (RX), but this is not guaranteed (many
sections can be placed before .text and they are not affected).
See https://reviews.llvm.org/D70468 "[ELF] Error if -Ttext-segment is specified"
Reviewed-by: Fangrui Song <maskray@google.com>
>>
>> commit 923ec3d20eef9e36456868b590873ce39f17fe71
>> Author: Ralf Baechle <ralf@linux-mips.org>
>> Date: Wed Nov 6 22:16:38 2002 +0000
>>
>> Define load address in linker script instead of relying on the
>> deprecated and notoriously unreliable option -Ttext.
>>
>> and previously `-Ttext' was used with this script anyway, though not
>> very long, as the script was entirely ignored until:
>>
>> commit 7a782968041ffc4c2d89816238e2f8ea5cceddba
>> Author: Ralf Baechle <ralf@linux-mips.org>
>> Date: Thu Oct 31 23:54:21 2002 +0000
>>
>> Merge with Linux 2.5.36.
>>
>> Maciej
>
>--
>Jiaxun Yang
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] MIPS: Truncate link address into 32bit for 32bit kernel
2020-04-13 15:34 ` Fangrui Song
@ 2020-04-13 20:08 ` Maciej W. Rozycki
0 siblings, 0 replies; 16+ messages in thread
From: Maciej W. Rozycki @ 2020-04-13 20:08 UTC (permalink / raw)
To: Fangrui Song
Cc: Jiaxun Yang, linux-mips, clang-built-linux, Nathan Chancellor,
Thomas Bogendoerfer, Borislav Petkov, Kees Cook, Heiko Carstens,
linux-kernel
On Mon, 13 Apr 2020, Fangrui Song wrote:
> >> > diff --git a/arch/mips/kernel/vmlinux.lds.S
> >> > b/arch/mips/kernel/vmlinux.lds.S index a5f00ec73ea6..5226cd8e4bee
> >> > 100644 --- a/arch/mips/kernel/vmlinux.lds.S
> >> > +++ b/arch/mips/kernel/vmlinux.lds.S
> >> > @@ -55,7 +55,7 @@ SECTIONS
> >> > /* . = 0xa800000000300000; */
> >> > . = 0xffffffff80300000;
> >> > #endif
> >> > - . = VMLINUX_LOAD_ADDRESS;
> >> > + . = VMLINUX_LINK_ADDRESS;
> >>
> >> The CONFIG_BOOT_ELF64 cruft right above it looks interesting to me,
> >> never have ever been used. We have had the current arrangement since:
> >
> >It confused me either.
> >It's only used by SGI so probably it's time to rename it as
> >BOOT_SEG_ELF64.
> >
> >Wish someone could clarify what is it.
> >
> >Thanks.
>
> Agreed that -Ttext in
>
> arch/mips/boot/compressed/Makefile
> 100: cmd_zld = $(LD) $(KBUILD_LDFLAGS) -Ttext $(VMLINUZ_LOAD_ADDRESS) -T
> $< $(vmlinuzobjs-y) -o $@
>
> and a few other places are brittle. They need to be replaced with Output
> Section Address:
> (https://sourceware.org/binutils/docs/ld/Output-Section-Address.html
> https://github.com/llvm/llvm-project/blob/master/lld/docs/ELF/linker_script.rst#output-section-address)
>
> -Ttext changes the address of .text . This can lead to the change of the
> address of the text segment (RX), but this is not guaranteed (many
> sections can be placed before .text and they are not affected).
That is unrelated, but you're free to clean it up of course.
Maciej
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] MIPS: Truncate link address into 32bit for 32bit kernel
2020-04-13 7:32 ` Jiaxun Yang
2020-04-13 15:34 ` Fangrui Song
@ 2020-04-13 20:06 ` Maciej W. Rozycki
1 sibling, 0 replies; 16+ messages in thread
From: Maciej W. Rozycki @ 2020-04-13 20:06 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-mips, clang-built-linux, Fangrui Song, Nathan Chancellor,
Thomas Bogendoerfer, Borislav Petkov, Kees Cook, Heiko Carstens,
linux-kernel
On Mon, 13 Apr 2020, Jiaxun Yang wrote:
> > > diff --git a/arch/mips/kernel/vmlinux.lds.S
> > > b/arch/mips/kernel/vmlinux.lds.S index a5f00ec73ea6..5226cd8e4bee
> > > 100644 --- a/arch/mips/kernel/vmlinux.lds.S
> > > +++ b/arch/mips/kernel/vmlinux.lds.S
> > > @@ -55,7 +55,7 @@ SECTIONS
> > > /* . = 0xa800000000300000; */
> > > . = 0xffffffff80300000;
> > > #endif
> > > - . = VMLINUX_LOAD_ADDRESS;
> > > + . = VMLINUX_LINK_ADDRESS;
> >
> > The CONFIG_BOOT_ELF64 cruft right above it looks interesting to me,
> > never have ever been used. We have had the current arrangement since:
>
> It confused me either.
> It's only used by SGI so probably it's time to rename it as
> BOOT_SEG_ELF64.
Well, as seen above the first assignment is immediately overridden by the
second and the rest of the conditional are comments. So as I say it's not
used ever, not even any SGI configuration, and from Git history it looks
like it has never been.
Maciej
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] MIPS: Truncate link address into 32bit for 32bit kernel
2020-04-13 6:26 [PATCH v4] MIPS: Truncate link address into 32bit for 32bit kernel Jiaxun Yang
2020-04-13 6:59 ` Maciej W. Rozycki
@ 2020-04-13 16:25 ` Kees Cook
2020-04-13 18:52 ` Nathan Chancellor
2020-04-22 14:32 ` [PATCH v5] " Jiaxun Yang
3 siblings, 0 replies; 16+ messages in thread
From: Kees Cook @ 2020-04-13 16:25 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-mips, macro, clang-built-linux, Fangrui Song,
Nathan Chancellor, Thomas Bogendoerfer, Borislav Petkov,
Heiko Carstens, linux-kernel
On Mon, Apr 13, 2020 at 02:26:49PM +0800, Jiaxun Yang wrote:
> LLD failed to link vmlinux with 64bit load address for 32bit ELF
> while bfd will strip 64bit address into 32bit silently.
> To fix LLD build, we should truncate load address provided by platform
> into 32bit for 32bit kernel.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Link: https://github.com/ClangBuiltLinux/linux/issues/786
> Link: https://sourceware.org/bugzilla/show_bug.cgi?id=25784
> Cc: Fangrui Song <maskray@google.com>
> Cc: Nathan Chancellor <natechancellor@gmail.com>
> --
> V2: Take MaskRay's shell magic.
>
> V3: After spent an hour on dealing with special character issue in
> Makefile, I gave up to do shell hacks and write a util in C instead.
> Thanks Maciej for pointing out Makefile variable problem.
>
> v4: Finally we managed to find a Makefile method to do it properly
> thanks to Kees. As it's too far from the initial version, I removed
> Review & Test tag from Nick and Fangrui and Cc instead.
> ---
> arch/mips/Makefile | 12 +++++++++++-
> arch/mips/kernel/vmlinux.lds.S | 2 +-
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index e1c44aed8156..18495568f03e 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -288,9 +288,19 @@ ifdef CONFIG_64BIT
> endif
> endif
>
> +# When linking a 32-bit executable the LLVM linker cannot cope with a
> +# 32-bit load address that has been sign-extended to 64 bits. Simply
> +# remove the upper 32 bits then, as it is safe to do so with other
> +# linkers.
> +ifdef CONFIG_64BIT
> + load-ld = $(load-y)
> +else
> + load-ld = $(subst 0xffffffff,0x,$(load-y))
> +endif
> +
> KBUILD_AFLAGS += $(cflags-y)
> KBUILD_CFLAGS += $(cflags-y)
> -KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y)
> +KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y) -DVMLINUX_LINK_ADDRESS=$(load-ld)
> KBUILD_CPPFLAGS += -DDATAOFFSET=$(if $(dataoffset-y),$(dataoffset-y),0)
>
> bootvars-y = VMLINUX_LOAD_ADDRESS=$(load-y) \
> diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
> index a5f00ec73ea6..5226cd8e4bee 100644
> --- a/arch/mips/kernel/vmlinux.lds.S
> +++ b/arch/mips/kernel/vmlinux.lds.S
> @@ -55,7 +55,7 @@ SECTIONS
> /* . = 0xa800000000300000; */
> . = 0xffffffff80300000;
> #endif
> - . = VMLINUX_LOAD_ADDRESS;
> + . = VMLINUX_LINK_ADDRESS;
> /* read-only */
> _text = .; /* Text and read-only data */
> .text : {
> --
> 2.26.0.rc2
>
--
Kees Cook
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v4] MIPS: Truncate link address into 32bit for 32bit kernel
2020-04-13 6:26 [PATCH v4] MIPS: Truncate link address into 32bit for 32bit kernel Jiaxun Yang
2020-04-13 6:59 ` Maciej W. Rozycki
2020-04-13 16:25 ` Kees Cook
@ 2020-04-13 18:52 ` Nathan Chancellor
2020-04-22 14:32 ` [PATCH v5] " Jiaxun Yang
3 siblings, 0 replies; 16+ messages in thread
From: Nathan Chancellor @ 2020-04-13 18:52 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-mips, macro, clang-built-linux, Fangrui Song,
Thomas Bogendoerfer, Borislav Petkov, Kees Cook, Heiko Carstens,
linux-kernel
Hi Jiaxun,
On Mon, Apr 13, 2020 at 02:26:49PM +0800, Jiaxun Yang wrote:
> LLD failed to link vmlinux with 64bit load address for 32bit ELF
> while bfd will strip 64bit address into 32bit silently.
> To fix LLD build, we should truncate load address provided by platform
> into 32bit for 32bit kernel.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/786
> Link: https://sourceware.org/bugzilla/show_bug.cgi?id=25784
> Cc: Fangrui Song <maskray@google.com>
> Cc: Nathan Chancellor <natechancellor@gmail.com>
> --
> V2: Take MaskRay's shell magic.
>
> V3: After spent an hour on dealing with special character issue in
> Makefile, I gave up to do shell hacks and write a util in C instead.
> Thanks Maciej for pointing out Makefile variable problem.
>
> v4: Finally we managed to find a Makefile method to do it properly
> thanks to Kees. As it's too far from the initial version, I removed
> Review & Test tag from Nick and Fangrui and Cc instead.
> ---
> arch/mips/Makefile | 12 +++++++++++-
> arch/mips/kernel/vmlinux.lds.S | 2 +-
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index e1c44aed8156..18495568f03e 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -288,9 +288,19 @@ ifdef CONFIG_64BIT
> endif
> endif
>
> +# When linking a 32-bit executable the LLVM linker cannot cope with a
> +# 32-bit load address that has been sign-extended to 64 bits. Simply
> +# remove the upper 32 bits then, as it is safe to do so with other
> +# linkers.
> +ifdef CONFIG_64BIT
> + load-ld = $(load-y)
> +else
> + load-ld = $(subst 0xffffffff,0x,$(load-y))
> +endif
> +
> KBUILD_AFLAGS += $(cflags-y)
> KBUILD_CFLAGS += $(cflags-y)
> -KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y)
> +KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y) -DVMLINUX_LINK_ADDRESS=$(load-ld)
> KBUILD_CPPFLAGS += -DDATAOFFSET=$(if $(dataoffset-y),$(dataoffset-y),0)
>
> bootvars-y = VMLINUX_LOAD_ADDRESS=$(load-y) \
> diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
> index a5f00ec73ea6..5226cd8e4bee 100644
> --- a/arch/mips/kernel/vmlinux.lds.S
> +++ b/arch/mips/kernel/vmlinux.lds.S
> @@ -55,7 +55,7 @@ SECTIONS
> /* . = 0xa800000000300000; */
> . = 0xffffffff80300000;
> #endif
> - . = VMLINUX_LOAD_ADDRESS;
> + . = VMLINUX_LINK_ADDRESS;
> /* read-only */
> _text = .; /* Text and read-only data */
> .text : {
> --
> 2.26.0.rc2
>
This version does not quite work:
ld.lld: error: section .text at 0xFFFFFFFF80990000 of size 0x388C exceeds available address space
ld.lld: error: section .MIPS.abiflags at 0xFFFFFFFF80993890 of size 0x18 exceeds available address space
ld.lld: error: section .rodata.str1.1 at 0xFFFFFFFF809938A8 of size 0x164 exceeds available address space
ld.lld: error: section .eh_frame at 0xFFFFFFFF80993A0C of size 0x2C exceeds available address space
ld.lld: error: section .data at 0xFFFFFFFF80993A40 of size 0x38EFD0 exceeds available address space
ld.lld: error: section .got at 0xFFFFFFFF80D22A10 of size 0x8 exceeds available address space
ld.lld: error: section .bss at 0xFFFFFFFF80E22A20 of size 0x402010 exceeds available address space
ld.lld: error: section .sbss at 0xFFFFFFFF81224A30 of size 0x8 exceeds available address space
make[2]: *** [/home/nathan/src/linux/arch/mips/boot/compressed/Makefile:104: vmlinuz] Error 1
I think something like this is needed but I do not know if it is proper.
Cheers,
Nathan
diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index 18495568f03e..68c0f22fefc0 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -304,6 +304,7 @@ KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y) -DVMLINUX_LINK_ADDRESS=$(loa
KBUILD_CPPFLAGS += -DDATAOFFSET=$(if $(dataoffset-y),$(dataoffset-y),0)
bootvars-y = VMLINUX_LOAD_ADDRESS=$(load-y) \
+ VMLINUX_LINK_ADDRESS=$(load-ld) \
VMLINUX_ENTRY_ADDRESS=$(entry-y) \
PLATFORM="$(platform-y)" \
ITS_INPUTS="$(its-y)"
diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile
index 0df0ee8a298d..3d391256ab7e 100644
--- a/arch/mips/boot/compressed/Makefile
+++ b/arch/mips/boot/compressed/Makefile
@@ -90,7 +90,7 @@ ifneq ($(zload-y),)
VMLINUZ_LOAD_ADDRESS := $(zload-y)
else
VMLINUZ_LOAD_ADDRESS = $(shell $(obj)/calc_vmlinuz_load_addr \
- $(obj)/vmlinux.bin $(VMLINUX_LOAD_ADDRESS))
+ $(obj)/vmlinux.bin $(VMLINUX_LINK_ADDRESS))
endif
UIMAGE_LOADADDR = $(VMLINUZ_LOAD_ADDRESS)
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v5] MIPS: Truncate link address into 32bit for 32bit kernel
2020-04-13 6:26 [PATCH v4] MIPS: Truncate link address into 32bit for 32bit kernel Jiaxun Yang
` (2 preceding siblings ...)
2020-04-13 18:52 ` Nathan Chancellor
@ 2020-04-22 14:32 ` Jiaxun Yang
2020-04-22 22:16 ` Nathan Chancellor
2020-04-23 0:10 ` Maciej W. Rozycki
3 siblings, 2 replies; 16+ messages in thread
From: Jiaxun Yang @ 2020-04-22 14:32 UTC (permalink / raw)
To: linux-mips
Cc: clang-built-linux, Jiaxun Yang, Maciej W . Rozycki, Fangrui Song,
Kees Cook, Nathan Chancellor, Thomas Bogendoerfer, Paul Burton,
Masahiro Yamada, Jouni Hogander, Kevin Darbyshire-Bryant,
Borislav Petkov, Heiko Carstens, linux-kernel
LLD failed to link vmlinux with 64bit load address for 32bit ELF
while bfd will strip 64bit address into 32bit silently.
To fix LLD build, we should truncate load address provided by platform
into 32bit for 32bit kernel.
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/786
Link: https://sourceware.org/bugzilla/show_bug.cgi?id=25784
Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>
Reviewed-by: Fangrui Song <maskray@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Cc: Nathan Chancellor <natechancellor@gmail.com>
--
V2: Take MaskRay's shell magic.
V3: After spent an hour on dealing with special character issue in
Makefile, I gave up to do shell hacks and write a util in C instead.
Thanks Maciej for pointing out Makefile variable problem.
v4: Finally we managed to find a Makefile method to do it properly
thanks to Kees. As it's too far from the initial version, I removed
Review & Test tag from Nick and Fangrui and Cc instead.
v5: Care vmlinuz as well.
---
arch/mips/Makefile | 13 ++++++++++++-
arch/mips/boot/compressed/Makefile | 2 +-
arch/mips/kernel/vmlinux.lds.S | 2 +-
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index e1c44aed8156..68c0f22fefc0 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -288,12 +288,23 @@ ifdef CONFIG_64BIT
endif
endif
+# When linking a 32-bit executable the LLVM linker cannot cope with a
+# 32-bit load address that has been sign-extended to 64 bits. Simply
+# remove the upper 32 bits then, as it is safe to do so with other
+# linkers.
+ifdef CONFIG_64BIT
+ load-ld = $(load-y)
+else
+ load-ld = $(subst 0xffffffff,0x,$(load-y))
+endif
+
KBUILD_AFLAGS += $(cflags-y)
KBUILD_CFLAGS += $(cflags-y)
-KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y)
+KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y) -DVMLINUX_LINK_ADDRESS=$(load-ld)
KBUILD_CPPFLAGS += -DDATAOFFSET=$(if $(dataoffset-y),$(dataoffset-y),0)
bootvars-y = VMLINUX_LOAD_ADDRESS=$(load-y) \
+ VMLINUX_LINK_ADDRESS=$(load-ld) \
VMLINUX_ENTRY_ADDRESS=$(entry-y) \
PLATFORM="$(platform-y)" \
ITS_INPUTS="$(its-y)"
diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile
index 0df0ee8a298d..3d391256ab7e 100644
--- a/arch/mips/boot/compressed/Makefile
+++ b/arch/mips/boot/compressed/Makefile
@@ -90,7 +90,7 @@ ifneq ($(zload-y),)
VMLINUZ_LOAD_ADDRESS := $(zload-y)
else
VMLINUZ_LOAD_ADDRESS = $(shell $(obj)/calc_vmlinuz_load_addr \
- $(obj)/vmlinux.bin $(VMLINUX_LOAD_ADDRESS))
+ $(obj)/vmlinux.bin $(VMLINUX_LINK_ADDRESS))
endif
UIMAGE_LOADADDR = $(VMLINUZ_LOAD_ADDRESS)
diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
index a5f00ec73ea6..5226cd8e4bee 100644
--- a/arch/mips/kernel/vmlinux.lds.S
+++ b/arch/mips/kernel/vmlinux.lds.S
@@ -55,7 +55,7 @@ SECTIONS
/* . = 0xa800000000300000; */
. = 0xffffffff80300000;
#endif
- . = VMLINUX_LOAD_ADDRESS;
+ . = VMLINUX_LINK_ADDRESS;
/* read-only */
_text = .; /* Text and read-only data */
.text : {
--
2.26.0.rc2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v5] MIPS: Truncate link address into 32bit for 32bit kernel
2020-04-22 14:32 ` [PATCH v5] " Jiaxun Yang
@ 2020-04-22 22:16 ` Nathan Chancellor
2020-04-23 0:10 ` Maciej W. Rozycki
1 sibling, 0 replies; 16+ messages in thread
From: Nathan Chancellor @ 2020-04-22 22:16 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-mips, clang-built-linux, Maciej W . Rozycki, Fangrui Song,
Kees Cook, Thomas Bogendoerfer, Paul Burton, Masahiro Yamada,
Jouni Hogander, Kevin Darbyshire-Bryant, Borislav Petkov,
Heiko Carstens, linux-kernel
On Wed, Apr 22, 2020 at 10:32:54PM +0800, Jiaxun Yang wrote:
> LLD failed to link vmlinux with 64bit load address for 32bit ELF
> while bfd will strip 64bit address into 32bit silently.
> To fix LLD build, we should truncate load address provided by platform
> into 32bit for 32bit kernel.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/786
> Link: https://sourceware.org/bugzilla/show_bug.cgi?id=25784
> Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>
> Reviewed-by: Fangrui Song <maskray@google.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Cc: Nathan Chancellor <natechancellor@gmail.com>
> --
> V2: Take MaskRay's shell magic.
>
> V3: After spent an hour on dealing with special character issue in
> Makefile, I gave up to do shell hacks and write a util in C instead.
> Thanks Maciej for pointing out Makefile variable problem.
>
> v4: Finally we managed to find a Makefile method to do it properly
> thanks to Kees. As it's too far from the initial version, I removed
> Review & Test tag from Nick and Fangrui and Cc instead.
>
> v5: Care vmlinuz as well.
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5] MIPS: Truncate link address into 32bit for 32bit kernel
2020-04-22 14:32 ` [PATCH v5] " Jiaxun Yang
2020-04-22 22:16 ` Nathan Chancellor
@ 2020-04-23 0:10 ` Maciej W. Rozycki
2020-04-23 5:42 ` Jiaxun Yang
1 sibling, 1 reply; 16+ messages in thread
From: Maciej W. Rozycki @ 2020-04-23 0:10 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-mips, clang-built-linux, Fangrui Song, Kees Cook,
Nathan Chancellor, Thomas Bogendoerfer, Paul Burton,
Masahiro Yamada, Jouni Hogander, Kevin Darbyshire-Bryant,
Borislav Petkov, Heiko Carstens, linux-kernel
On Wed, 22 Apr 2020, Jiaxun Yang wrote:
> Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>
Hmm, that was for an earlier version of the patch, and reviews obviously
do not automatically carry over to subsequent versions, as it cannot be
claimed that they are as good in the reviewer's eyes as the actual version
reviewed was.
> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index e1c44aed8156..68c0f22fefc0 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -288,12 +288,23 @@ ifdef CONFIG_64BIT
> endif
> endif
>
> +# When linking a 32-bit executable the LLVM linker cannot cope with a
> +# 32-bit load address that has been sign-extended to 64 bits. Simply
> +# remove the upper 32 bits then, as it is safe to do so with other
> +# linkers.
> +ifdef CONFIG_64BIT
> + load-ld = $(load-y)
> +else
> + load-ld = $(subst 0xffffffff,0x,$(load-y))
> +endif
> +
> KBUILD_AFLAGS += $(cflags-y)
> KBUILD_CFLAGS += $(cflags-y)
> -KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y)
> +KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y) -DVMLINUX_LINK_ADDRESS=$(load-ld)
> KBUILD_CPPFLAGS += -DDATAOFFSET=$(if $(dataoffset-y),$(dataoffset-y),0)
>
> bootvars-y = VMLINUX_LOAD_ADDRESS=$(load-y) \
> + VMLINUX_LINK_ADDRESS=$(load-ld) \
> VMLINUX_ENTRY_ADDRESS=$(entry-y) \
> PLATFORM="$(platform-y)" \
> ITS_INPUTS="$(its-y)"
Hmm, to be honest I find the nomenclature confusing: VMLINUX_LOAD_ADDRESS
and VMLINUX_LINK_ADDRESS sound like synonyms to me and also look very
similar, so I expect people will be confused and scratch their heads in
the future. Due to the obscurity of the problem I think there is little
room for manoeuvre here really, but how about using LINKER_LOAD_ADDRESS
for the new variable?
Alternatively, have you made any attempt to verify if actually replacing
the setting for VMLINUX_LOAD_ADDRESS would be safe? Glancing over its use
there do not appear to be many places.
Maciej
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5] MIPS: Truncate link address into 32bit for 32bit kernel
2020-04-23 0:10 ` Maciej W. Rozycki
@ 2020-04-23 5:42 ` Jiaxun Yang
2020-04-24 12:22 ` Maciej W. Rozycki
0 siblings, 1 reply; 16+ messages in thread
From: Jiaxun Yang @ 2020-04-23 5:42 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: linux-mips, clang-built-linux, Fangrui Song, Kees Cook,
Nathan Chancellor, Thomas Bogendoerfer, Paul Burton,
Masahiro Yamada, Jouni Hogander, Kevin Darbyshire-Bryant,
Borislav Petkov, Heiko Carstens, linux-kernel
于 2020年4月23日 GMT+08:00 上午8:10:12, "Maciej W. Rozycki" <macro@linux-mips.org> 写到:
>On Wed, 22 Apr 2020, Jiaxun Yang wrote:
>
>> Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>
>
> Hmm, that was for an earlier version of the patch, and reviews obviously
>do not automatically carry over to subsequent versions, as it cannot be
>claimed that they are as good in the reviewer's eyes as the actual version
>reviewed was.
>
>> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
>> index e1c44aed8156..68c0f22fefc0 100644
>> --- a/arch/mips/Makefile
>> +++ b/arch/mips/Makefile
>> @@ -288,12 +288,23 @@ ifdef CONFIG_64BIT
>> endif
>> endif
>>
>> +# When linking a 32-bit executable the LLVM linker cannot cope with a
>> +# 32-bit load address that has been sign-extended to 64 bits. Simply
>> +# remove the upper 32 bits then, as it is safe to do so with other
>> +# linkers.
>> +ifdef CONFIG_64BIT
>> + load-ld = $(load-y)
>> +else
>> + load-ld = $(subst 0xffffffff,0x,$(load-y))
>> +endif
>> +
>> KBUILD_AFLAGS += $(cflags-y)
>> KBUILD_CFLAGS += $(cflags-y)
>> -KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y)
>> +KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y) -DVMLINUX_LINK_ADDRESS=$(load-ld)
>> KBUILD_CPPFLAGS += -DDATAOFFSET=$(if $(dataoffset-y),$(dataoffset-y),0)
>>
>> bootvars-y = VMLINUX_LOAD_ADDRESS=$(load-y) \
>> + VMLINUX_LINK_ADDRESS=$(load-ld) \
>> VMLINUX_ENTRY_ADDRESS=$(entry-y) \
>> PLATFORM="$(platform-y)" \
>> ITS_INPUTS="$(its-y)"
>
> Hmm, to be honest I find the nomenclature confusing: VMLINUX_LOAD_ADDRESS
>and VMLINUX_LINK_ADDRESS sound like synonyms to me and also look very
>similar, so I expect people will be confused and scratch their heads in
>the future. Due to the obscurity of the problem I think there is little
>room for manoeuvre here really, but how about using LINKER_LOAD_ADDRESS
>for the new variable?
>
> Alternatively, have you made any attempt to verify if actually replacing
>the setting for VMLINUX_LOAD_ADDRESS would be safe? Glancing over its use
>there do not appear to be many places.
Limited experiments showed it should be fine...
But MIPS kernel has some design I'm not really familiar with like SYM32 for
64-bit kernel and special address space design for Trap-and-emul KVM.
I'm not 100 sure it's safe so I didn't do so.
>
> Maciej
--
Jiaxun Yang
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5] MIPS: Truncate link address into 32bit for 32bit kernel
2020-04-23 5:42 ` Jiaxun Yang
@ 2020-04-24 12:22 ` Maciej W. Rozycki
2020-05-04 15:46 ` Thomas Bogendoerfer
0 siblings, 1 reply; 16+ messages in thread
From: Maciej W. Rozycki @ 2020-04-24 12:22 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-mips, clang-built-linux, Fangrui Song, Kees Cook,
Nathan Chancellor, Thomas Bogendoerfer, Paul Burton,
Masahiro Yamada, Jouni Hogander, Kevin Darbyshire-Bryant,
Borislav Petkov, Heiko Carstens, linux-kernel
On Thu, 23 Apr 2020, Jiaxun Yang wrote:
> > Alternatively, have you made any attempt to verify if actually replacing
> >the setting for VMLINUX_LOAD_ADDRESS would be safe? Glancing over its use
> >there do not appear to be many places.
>
> Limited experiments showed it should be fine...
>
> But MIPS kernel has some design I'm not really familiar with like SYM32 for
> 64-bit kernel and special address space design for Trap-and-emul KVM.
This only affects CONFIG_32BIT kernels, so SYM32 does not apply; I can't
comment on KVM. There's still that bunch of:
$(shell expr $(...) \< 0xffffffff80000000)
constructs I mentioned before, so let's leave your change as it stands at
this time. Please do rename the variable as I suggested though, I hope
that's not a big deal.
Maciej
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5] MIPS: Truncate link address into 32bit for 32bit kernel
2020-04-24 12:22 ` Maciej W. Rozycki
@ 2020-05-04 15:46 ` Thomas Bogendoerfer
2020-05-04 16:09 ` Jiaxun Yang
0 siblings, 1 reply; 16+ messages in thread
From: Thomas Bogendoerfer @ 2020-05-04 15:46 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Jiaxun Yang, linux-mips, clang-built-linux, Fangrui Song,
Kees Cook, Nathan Chancellor, Paul Burton, Masahiro Yamada,
Jouni Hogander, Kevin Darbyshire-Bryant, Borislav Petkov,
Heiko Carstens, linux-kernel
On Fri, Apr 24, 2020 at 01:22:30PM +0100, Maciej W. Rozycki wrote:
> On Thu, 23 Apr 2020, Jiaxun Yang wrote:
>
> > > Alternatively, have you made any attempt to verify if actually replacing
> > >the setting for VMLINUX_LOAD_ADDRESS would be safe? Glancing over its use
> > >there do not appear to be many places.
> >
> > Limited experiments showed it should be fine...
> >
> > But MIPS kernel has some design I'm not really familiar with like SYM32 for
> > 64-bit kernel and special address space design for Trap-and-emul KVM.
>
> This only affects CONFIG_32BIT kernels, so SYM32 does not apply; I can't
> comment on KVM. There's still that bunch of:
>
> $(shell expr $(...) \< 0xffffffff80000000)
>
> constructs I mentioned before, so let's leave your change as it stands at
> this time. Please do rename the variable as I suggested though, I hope
> that's not a big deal.
Jiaxun, are you going to send an update with this change ?
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5] MIPS: Truncate link address into 32bit for 32bit kernel
2020-05-04 15:46 ` Thomas Bogendoerfer
@ 2020-05-04 16:09 ` Jiaxun Yang
2020-05-04 16:56 ` Thomas Bogendoerfer
0 siblings, 1 reply; 16+ messages in thread
From: Jiaxun Yang @ 2020-05-04 16:09 UTC (permalink / raw)
To: Thomas Bogendoerfer, Maciej W. Rozycki
Cc: linux-mips, clang-built-linux, Fangrui Song, Kees Cook,
Nathan Chancellor, Paul Burton, Masahiro Yamada, Jouni Hogander,
Kevin Darbyshire-Bryant, Borislav Petkov, Heiko Carstens,
linux-kernel
于 2020年5月4日 GMT+08:00 下午11:46:13, Thomas Bogendoerfer <tsbogend@alpha.franken.de> 写到:
>On Fri, Apr 24, 2020 at 01:22:30PM +0100, Maciej W. Rozycki wrote:
>> On Thu, 23 Apr 2020, Jiaxun Yang wrote:
>>
>> > > Alternatively, have you made any attempt to verify if actually replacing
>> > >the setting for VMLINUX_LOAD_ADDRESS would be safe? Glancing over its use
>> > >there do not appear to be many places.
>> >
>> > Limited experiments showed it should be fine...
>> >
>> > But MIPS kernel has some design I'm not really familiar with like SYM32 for
>> > 64-bit kernel and special address space design for Trap-and-emul KVM.
>>
>> This only affects CONFIG_32BIT kernels, so SYM32 does not apply; I can't
>> comment on KVM. There's still that bunch of:
>>
>> $(shell expr $(...) \< 0xffffffff80000000)
>>
>> constructs I mentioned before, so let's leave your change as it stands at
>> this time. Please do rename the variable as I suggested though, I hope
>> that's not a big deal.
>
>Jiaxun, are you going to send an update with this change ?
Sorry my mail server missed Maciej's reply.
Should I send another version or you just fix it at apply time?
>
>Thomas.
>
--
Jiaxun Yang
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5] MIPS: Truncate link address into 32bit for 32bit kernel
2020-05-04 16:09 ` Jiaxun Yang
@ 2020-05-04 16:56 ` Thomas Bogendoerfer
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Bogendoerfer @ 2020-05-04 16:56 UTC (permalink / raw)
To: Jiaxun Yang
Cc: Maciej W. Rozycki, linux-mips, clang-built-linux, Fangrui Song,
Kees Cook, Nathan Chancellor, Paul Burton, Masahiro Yamada,
Jouni Hogander, Kevin Darbyshire-Bryant, Borislav Petkov,
Heiko Carstens, linux-kernel
On Tue, May 05, 2020 at 12:09:46AM +0800, Jiaxun Yang wrote:
>
> 于 2020年5月4日 GMT+08:00 下午11:46:13, Thomas Bogendoerfer <tsbogend@alpha.franken.de> 写到:
> >On Fri, Apr 24, 2020 at 01:22:30PM +0100, Maciej W. Rozycki wrote:
> >> On Thu, 23 Apr 2020, Jiaxun Yang wrote:
> >>
> >> > > Alternatively, have you made any attempt to verify if actually replacing
> >> > >the setting for VMLINUX_LOAD_ADDRESS would be safe? Glancing over its use
> >> > >there do not appear to be many places.
> >> >
> >> > Limited experiments showed it should be fine...
> >> >
> >> > But MIPS kernel has some design I'm not really familiar with like SYM32 for
> >> > 64-bit kernel and special address space design for Trap-and-emul KVM.
> >>
> >> This only affects CONFIG_32BIT kernels, so SYM32 does not apply; I can't
> >> comment on KVM. There's still that bunch of:
> >>
> >> $(shell expr $(...) \< 0xffffffff80000000)
> >>
> >> constructs I mentioned before, so let's leave your change as it stands at
> >> this time. Please do rename the variable as I suggested though, I hope
> >> that's not a big deal.
> >
> >Jiaxun, are you going to send an update with this change ?
>
> Sorry my mail server missed Maciej's reply.
>
> Should I send another version or you just fix it at apply time?
please send a new version, thank you.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 16+ messages in thread