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 <[email protected]>
Link: https://github.com/ClangBuiltLinux/linux/issues/786
Link: https://sourceware.org/bugzilla/show_bug.cgi?id=25784
Reviewed-by: Maciej W. Rozycki <[email protected]>
Reviewed-by: Fangrui Song <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Cc: Nathan Chancellor <[email protected]>
--
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
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 <[email protected]>
> Link: https://github.com/ClangBuiltLinux/linux/issues/786
> Link: https://sourceware.org/bugzilla/show_bug.cgi?id=25784
> Reviewed-by: Maciej W. Rozycki <[email protected]>
> Reviewed-by: Fangrui Song <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
> Cc: Nathan Chancellor <[email protected]>
> --
> 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 <[email protected]>
于 2020年4月23日 GMT+08:00 上午8:10:12, "Maciej W. Rozycki" <[email protected]> 写到:
>On Wed, 22 Apr 2020, Jiaxun Yang wrote:
>
>> Reviewed-by: Maciej W. Rozycki <[email protected]>
>
> 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
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 ]
On Tue, May 05, 2020 at 12:09:46AM +0800, Jiaxun Yang wrote:
>
> 于 2020年5月4日 GMT+08:00 下午11:46:13, Thomas Bogendoerfer <[email protected]> 写到:
> >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 ]
于 2020年5月4日 GMT+08:00 下午11:46:13, Thomas Bogendoerfer <[email protected]> 写到:
>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
On Wed, 22 Apr 2020, Jiaxun Yang wrote:
> Reviewed-by: Maciej W. Rozycki <[email protected]>
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
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