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

[Andrew Morton]

On Mon, 22 Apr 2019 15:57:27 -0700 Kees Cook <keescook@xxxxxxxxxxxx> 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
_