Skip to content

Commit 4ed2863

Browse files
Michal Hockotorvalds
authored andcommitted
fs, elf: drop MAP_FIXED usage from elf_map
Both load_elf_interp and load_elf_binary rely on elf_map to map segments on a controlled address and they use MAP_FIXED to enforce that. This is however dangerous thing prone to silent data corruption which can be even exploitable. Let's take CVE-2017-1000253 as an example. At the time (before commit eab0953: "binfmt_elf: use ELF_ET_DYN_BASE only for PIE") ELF_ET_DYN_BASE was at TASK_SIZE / 3 * 2 which is not that far away from the stack top on 32b (legacy) memory layout (only 1GB away). Therefore we could end up mapping over the existing stack with some luck. The issue has been fixed since then (a87938b: "fs/binfmt_elf.c: fix bug in loading of PIE binaries"), ELF_ET_DYN_BASE moved moved much further from the stack (eab0953 and later by c715b72: "mm: revert x86_64 and arm64 ELF_ET_DYN_BASE base changes") and excessive stack consumption early during execve fully stopped by da029c1 ("exec: Limit arg stack to at most 75% of _STK_LIM"). So we should be safe and any attack should be impractical. On the other hand this is just too subtle assumption so it can break quite easily and hard to spot. I believe that the MAP_FIXED usage in load_elf_binary (et. al) is still fundamentally dangerous. Moreover it shouldn't be even needed. We are at the early process stage and so there shouldn't be unrelated mappings (except for stack and loader) existing so mmap for a given address should succeed even without MAP_FIXED. Something is terribly wrong if this is not the case and we should rather fail than silently corrupt the underlying mapping. Address this issue by changing MAP_FIXED to the newly added MAP_FIXED_NOREPLACE. This will mean that mmap will fail if there is an existing mapping clashing with the requested one without clobbering it. [[email protected]: fix build] [[email protected]: coding-style fixes] [[email protected]: don't use the same value for MAP_FIXED_NOREPLACE and MAP_SYNC] Link: http://lkml.kernel.org/r/[email protected] Link: http://lkml.kernel.org/r/[email protected] Signed-off-by: Michal Hocko <[email protected]> Signed-off-by: Andrei Vagin <[email protected]> Signed-off-by: Michal Hocko <[email protected]> Reviewed-by: Khalid Aziz <[email protected]> Acked-by: Michael Ellerman <[email protected]> Acked-by: Kees Cook <[email protected]> Cc: Abdul Haleem <[email protected]> Cc: Joel Stanley <[email protected]> Cc: Anshuman Khandual <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent a4ff8e8 commit 4ed2863

File tree

2 files changed

+12
-5
lines changed

2 files changed

+12
-5
lines changed

fs/binfmt_elf.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,11 @@ static unsigned long elf_map(struct file *filep, unsigned long addr,
377377
} else
378378
map_addr = vm_mmap(filep, addr, size, prot, type, off);
379379

380+
if ((type & MAP_FIXED_NOREPLACE) && BAD_ADDR(map_addr))
381+
pr_info("%d (%s): Uhuuh, elf segment at %p requested but the memory is mapped already\n",
382+
task_pid_nr(current), current->comm,
383+
(void *)addr);
384+
380385
return(map_addr);
381386
}
382387

@@ -575,7 +580,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
575580
elf_prot |= PROT_EXEC;
576581
vaddr = eppnt->p_vaddr;
577582
if (interp_elf_ex->e_type == ET_EXEC || load_addr_set)
578-
elf_type |= MAP_FIXED;
583+
elf_type |= MAP_FIXED_NOREPLACE;
579584
else if (no_base && interp_elf_ex->e_type == ET_DYN)
580585
load_addr = -vaddr;
581586

@@ -939,7 +944,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
939944
* the ET_DYN load_addr calculations, proceed normally.
940945
*/
941946
if (loc->elf_ex.e_type == ET_EXEC || load_addr_set) {
942-
elf_flags |= MAP_FIXED;
947+
elf_flags |= MAP_FIXED_NOREPLACE;
943948
} else if (loc->elf_ex.e_type == ET_DYN) {
944949
/*
945950
* This logic is run once for the first LOAD Program
@@ -975,7 +980,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
975980
load_bias = ELF_ET_DYN_BASE;
976981
if (current->flags & PF_RANDOMIZE)
977982
load_bias += arch_mmap_rnd();
978-
elf_flags |= MAP_FIXED;
983+
elf_flags |= MAP_FIXED_NOREPLACE;
979984
} else
980985
load_bias = 0;
981986

@@ -1235,7 +1240,7 @@ static int load_elf_library(struct file *file)
12351240
(eppnt->p_filesz +
12361241
ELF_PAGEOFFSET(eppnt->p_vaddr)),
12371242
PROT_READ | PROT_WRITE | PROT_EXEC,
1238-
MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE,
1243+
MAP_FIXED_NOREPLACE | MAP_PRIVATE | MAP_DENYWRITE,
12391244
(eppnt->p_offset -
12401245
ELF_PAGEOFFSET(eppnt->p_vaddr)));
12411246
if (error != ELF_PAGESTART(eppnt->p_vaddr))

include/uapi/asm-generic/mman-common.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626
#else
2727
# define MAP_UNINITIALIZED 0x0 /* Don't support this flag */
2828
#endif
29-
#define MAP_FIXED_NOREPLACE 0x80000 /* MAP_FIXED which doesn't unmap underlying mapping */
29+
30+
/* 0x0100 - 0x80000 flags are defined in asm-generic/mman.h */
31+
#define MAP_FIXED_NOREPLACE 0x100000 /* MAP_FIXED which doesn't unmap underlying mapping */
3032

3133
/*
3234
* Flags for mlock

0 commit comments

Comments
 (0)