linux-kernel - Re: [PATCH v5] MIPS: Truncate link address into 32bit for 32bit kernel

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.21.2004230036480.851719@eddie.linux-mips.org>
Date:   Thu, 23 Apr 2020 01:10:12 +0100 (BST)
From:   "Maciej W. Rozycki" <macro@...ux-mips.org>
To:     Jiaxun Yang <jiaxun.yang@...goat.com>
cc:     linux-mips@...r.kernel.org, clang-built-linux@...glegroups.com,
        Fangrui Song <maskray@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        Nathan Chancellor <natechancellor@...il.com>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Paul Burton <paulburton@...nel.org>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Jouni Hogander <jouni.hogander@...kie.com>,
        Kevin Darbyshire-Bryant <ldir@...byshire-bryant.me.uk>,
        Borislav Petkov <bp@...e.de>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5] MIPS: Truncate link address into 32bit for 32bit
 kernel

On Wed, 22 Apr 2020, Jiaxun Yang wrote:

> Reviewed-by: Maciej W. Rozycki <macro@...ux-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