linux-kernel - Re: [PATCH v2] binfmt_elf: Move brk out of mmap when doing direct loader exec

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20190422162405.503d0d068b880f87dc7bc889@linux-foundation.org>
Date:   Mon, 22 Apr 2019 16:24:05 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     Ali Saidi <alisaidi@...zon.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Michal Hocko <mhocko@...e.com>,
        Matthew Wilcox <willy@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jann Horn <jannh@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] binfmt_elf: Move brk out of mmap when doing direct
 loader exec

On Mon, 22 Apr 2019 15:57:27 -0700 Kees Cook <keescook@...omium.org> wrote:

> Commit eab09532d400 ("binfmt_elf: use ELF_ET_DYN_BASE only for PIE"),
> made changes in the rare case when the ELF loader was directly invoked
> (e.g to set a non-inheritable LD_LIBRARY_PATH, testing new versions of
> the loader), by moving into the mmap region to avoid both ET_EXEC and PIE
> binaries. This had the effect of also moving the brk region into mmap,
> which could lead to the stack and brk being arbitrarily close to each
> other. An unlucky process wouldn't get its requested stack size and stack
> allocations could end up scribbling on the heap.
>
> ...
>
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1131,16 +1131,18 @@ static int load_elf_binary(struct linux_binprm *bprm)
>  	current->mm->end_data = end_data;
>  	current->mm->start_stack = bprm->p;
>  
> -	/*
> -	 * When executing a loader directly (ET_DYN without Interp), move
> -	 * the brk area out of the mmap region (since it grows up, and may
> -	 * collide early with the stack growing down), and into the unused
> -	 * ELF_ET_DYN_BASE region.
> -	 */
> -	if (!interpreter)
> -		current->mm->brk = current->mm->start_brk = ELF_ET_DYN_BASE;

The above bit isn't there any more.  Here's what I queued:

--- a/fs/binfmt_elf.c~binfmt_elf-move-brk-out-of-mmap-when-doing-direct-loader-exec
+++ a/fs/binfmt_elf.c
@@ -1134,6 +1134,17 @@ out_free_interp:
 	current->mm->start_stack = bprm->p;
 
 	if ((current->flags & PF_RANDOMIZE) && (randomize_va_space > 1)) {
+		/*
+		 * For architectures with ELF randomization, when executing
+		 * a loader directly (i.e. no interpreter listed in ELF
+		 * headers), move the brk area out of the mmap region
+		 * (since it grows up, and may collide early with the stack
+		 * growing down), and into the unused ELF_ET_DYN_BASE region.
+		 */
+		if (IS_ENABLED(CONFIG_ARCH_HAS_ELF_RANDOMIZE) && !interpreter)
+			current->mm->brk = current->mm->start_brk =
+				ELF_ET_DYN_BASE;
+
 		current->mm->brk = current->mm->start_brk =
 			arch_randomize_brk(current->mm);
 #ifdef compat_brk_randomized
_