| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <202004101334.A641706F0@keescook>
Date: Fri, 10 Apr 2020 13:45:39 -0700
From: Kees Cook <keescook@...omium.org>
To: Jiaxun Yang <jiaxun.yang@...goat.com>
Cc: linux-mips@...r.kernel.org, macro@...ux-mips.org,
Fangrui Song <maskray@...gle.com>,
Nathan Chancellor <natechancellor@...il.com>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Paul Burton <paulburton@...nel.org>,
Borislav Petkov <bp@...e.de>,
Heiko Carstens <heiko.carstens@...ibm.com>,
Masahiro Yamada <masahiroy@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-kernel@...r.kernel.org, clang-built-linux@...glegroups.com
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@...goat.com>
> Reviewed-by: Fangrui Song <maskray@...gle.com>
> Tested-by: Nathan Chancellor <natechancellor@...il.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
Powered by blists - more mailing lists