Messages in this thread Patch in this message | | | Date | Fri, 10 Apr 2020 13:45:39 -0700 | From | Kees Cook <> | Subject | Re: [PATCH v3] MIPS: Truncate link address into 32bit for 32bit kernel |
| |
On Fri, Apr 10, 2020 at 05:06:23PM +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: Fangrui Song <maskray@google.com> > Tested-by: 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. > --- > arch/mips/Makefile | 2 ++ > arch/mips/kernel/Makefile | 11 ++++++++++- > arch/mips/kernel/vmlinux.lds.S | 2 +- > arch/mips/tools/.gitignore | 1 + > arch/mips/tools/Makefile | 5 +++++ > arch/mips/tools/truncate32.c | 29 +++++++++++++++++++++++++++++ > 6 files changed, 48 insertions(+), 2 deletions(-) > create mode 100644 arch/mips/tools/truncate32.c > > diff --git a/arch/mips/Makefile b/arch/mips/Makefile > index e1c44aed8156..633e9de4d262 100644 > --- a/arch/mips/Makefile > +++ b/arch/mips/Makefile > @@ -14,6 +14,7 @@ > > archscripts: scripts_basic > $(Q)$(MAKE) $(build)=arch/mips/tools elf-entry > + $(Q)$(MAKE) $(build)=arch/mips/tools truncate32 > ifeq ($(CONFIG_CPU_LOONGSON3_WORKAROUNDS),y) > $(Q)$(MAKE) $(build)=arch/mips/tools loongson3-llsc-check > endif > @@ -261,6 +262,7 @@ include arch/mips/Kbuild.platforms > ifdef CONFIG_PHYSICAL_START > load-y = $(CONFIG_PHYSICAL_START) > endif > +export VMLINUX_LOAD_ADDRESS := $(load-y) > > entry-y = $(shell $(objtree)/arch/mips/tools/elf-entry vmlinux) > cflags-y += -I$(srctree)/arch/mips/include/asm/mach-generic > diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile > index d6e97df51cfb..0178f7085317 100644 > --- a/arch/mips/kernel/Makefile > +++ b/arch/mips/kernel/Makefile > @@ -112,4 +112,13 @@ obj-$(CONFIG_MIPS_CPC) += mips-cpc.o > obj-$(CONFIG_CPU_PM) += pm.o > obj-$(CONFIG_MIPS_CPS_PM) += pm-cps.o > > -CPPFLAGS_vmlinux.lds := $(KBUILD_CFLAGS) > +# 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 = $(VMLINUX_LOAD_ADDRESS) > +else > + load-ld = $(shell $(objtree)/arch/mips/tools/truncate32 $(VMLINUX_LOAD_ADDRESS))
This is major overkill. Just use the Makefile's builtin text manipulation:
load-ld = $(subst 0xffffffff,0x,$(VMLINUX_LOAD_ADDRESS))
And note that a shell failure here would be entirely ignored, so the use of die() in the proposed helper wouldn't actually stop a build, etc.
> +endif > +CPPFLAGS_vmlinux.lds := $(KBUILD_CFLAGS) -DVMLINUX_LINK_ADDRESS=$(load-ld) > 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 : { > diff --git a/arch/mips/tools/.gitignore b/arch/mips/tools/.gitignore > index 794817dfb389..58ead412c8d3 100644 > --- a/arch/mips/tools/.gitignore > +++ b/arch/mips/tools/.gitignore > @@ -1,3 +1,4 @@ > # SPDX-License-Identifier: GPL-2.0-only > elf-entry > loongson3-llsc-check > +truncate32 > diff --git a/arch/mips/tools/Makefile b/arch/mips/tools/Makefile > index b851e5dcc65a..69debb18bbb4 100644 > --- a/arch/mips/tools/Makefile > +++ b/arch/mips/tools/Makefile > @@ -8,3 +8,8 @@ hostprogs += loongson3-llsc-check > PHONY += loongson3-llsc-check > loongson3-llsc-check: $(obj)/loongson3-llsc-check > @: > + > +hostprogs += truncate32 > +PHONY += truncate32
Isn't the special variable named ".PHONY"? (And also is that only for things the don't get written to disk, but truncate32 is...)
> +truncate32: $(obj)/truncate32 > + @: > diff --git a/arch/mips/tools/truncate32.c b/arch/mips/tools/truncate32.c > new file mode 100644 > index 000000000000..82c19b4c32da > --- /dev/null > +++ b/arch/mips/tools/truncate32.c > @@ -0,0 +1,29 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <stdint.h> > +#include <stdio.h> > +#include <stdlib.h> > + > +__attribute__((noreturn)) > +static void die(const char *msg) > +{ > + fputs(msg, stderr); > + exit(EXIT_FAILURE); > +} > + > +int main(int argc, const char *argv[]) > +{ > + unsigned long long val; > + > + if (argc != 2) > + die("Usage: truncate32 <address>\n"); > + > + val = strtoull(argv[1], NULL, 0); > + > + if ((val & 0xffffffff00000000) != 0xffffffff00000000) > + die("Invalid input\n"); > + > + val = val & 0xffffffff; > + printf("0x%08llx\n", val); > + > + return EXIT_SUCCESS; > +} > -- > 2.26.0.rc2
-- Kees Cook
| |