linux-kernel - Re: [PATCH v1] Arm64: ASLR: fix text randomization

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: <20141007134349.GR24725@leverpostej>
Date:	Tue, 7 Oct 2014 14:43:50 +0100
From:	Mark Rutland <mark.rutland@....com>
To:	Arun Chandran <achandran@...sta.com>
Cc:	Catalin Marinas <Catalin.Marinas@....com>,
	Will Deacon <Will.Deacon@....com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Anton Blanchard <anton@...ba.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Paul Mackerras <paulus@...ba.org>,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	Martin Schwidefsky <schwidefsky@...ibm.com>
Subject: Re: [PATCH v1] Arm64: ASLR: fix text randomization

On Tue, Oct 07, 2014 at 01:40:28PM +0100, Arun Chandran wrote:
> This is due to incorrect definition of ELF_ET_DYN_BASE. It
> introduces randomization for text even if user does a "echo 0 >
> /proc/sys/kernel/randomize_va_space"

Interesting.

It looks like this was a copy of what powerpc and s390 do (authors
Cc'd), and the generic support came later. powerpc gained support in
501cb16d3cfdcca9 (powerpc: Randomise PIEs), but the generic support was
enabled later in e39f560239984c30 (fs: binfmt_elf: create Kconfig
variable for PIE randomization).

The policy of disabling PIE randomization was added in a3defbe5c337dbc6
(binfmt_elf: fix PIE execution with randomization disabled), after the
powerpc implementation, but before the x86 implementation was made
generic.

I wasn't able to spot where the randomness came from in the
ARCH_BINFMT_ELF_RANDOMIZE_PIE case, so it's not clear to me if the
generic implementation behaves identically other than disabling
randomization when told to via proc.

Assuming it behaves similarly enough, it looks like arm64, powerpc, and
s390 should all be moved over.

> 
> Signed-off-by: Arun Chandran <achandran@...sta.com>
> ---
> This can be tested using the code below
> 
> #include <stdio.h>
> 
> int main(int argc, char *argv)
> {
>     printf("main = %p\n", main);
>     return 0;
> }
> 
> * compile it possition independently 
>   aarch64-linux-gnu-gcc -fPIE -pie aslr.c -o aslr
> 
> * run it on the target
> 
> 	# ./aslr
> 	main = 0x7f87138950
> 	# ./aslr
> 	main = 0x7f94a10950
> 	# ./aslr
> 	main = 0x7f94fee950
> 	# ./aslr text
> 	main = 0x7f8cb72950
> 
> 	# echo 0 > /proc/sys/kernel/randomize_va_space 
> 	# ./aslr text
> 	main = 0x5555555950
> 	# ./aslr 
> 	main = 0x5555555950
> 	# ./aslr 
> 	main = 0x5555555950
> 	# ./aslr 
> 	main = 0x5555555950

It would be worth pointing out that this is after your patch is applied.
Before your patch I get randomized VAs even after writing 0 to
randomize_va_spave.

> ---
>  arch/arm64/Kconfig           |    1 +
>  arch/arm64/include/asm/elf.h |    4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index fd4e81a..a2eefc9 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1,5 +1,6 @@
>  config ARM64
>  	def_bool y
> +	select ARCH_BINFMT_ELF_RANDOMIZE_PIE
>  	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>  	select ARCH_HAS_SG_CHAIN
>  	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> index 01d3aab..1f65be3 100644
> --- a/arch/arm64/include/asm/elf.h
> +++ b/arch/arm64/include/asm/elf.h
> @@ -126,7 +126,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
>   * that it will "exec", and that there is sufficient room for the brk.
>   */
>  extern unsigned long randomize_et_dyn(unsigned long base);
> -#define ELF_ET_DYN_BASE	(randomize_et_dyn(2 * TASK_SIZE_64 / 3))
> +#define ELF_ET_DYN_BASE	(2 * TASK_SIZE_64 / 3)
>  
>  /*
>   * When the program starts, a1 contains a pointer to a function to be
> @@ -169,7 +169,7 @@ extern unsigned long arch_randomize_brk(struct mm_struct *mm);
>  #define COMPAT_ELF_PLATFORM		("v8l")
>  #endif
>  
> -#define COMPAT_ELF_ET_DYN_BASE		(randomize_et_dyn(2 * TASK_SIZE_32 / 3))
> +#define COMPAT_ELF_ET_DYN_BASE		(2 * TASK_SIZE_32 / 3)
>  
>  /* AArch32 registers. */
>  #define COMPAT_ELF_NGREG		18
> -- 
> 1.7.9.5

Given randomize_et_dyn is no longer used after this patch, I think it
should be dropped from arch/arm64/kernel/process.c.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/