On Mon, 22 Apr 2019 15:57:27 -0700 Kees Cook <[email protected]> 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
_
