summaryrefslogtreecommitdiffstats
path: root/fs
diff options
context:
space:
mode:
authorMichal Hocko <mhocko@suse.com>2018-04-10 19:36:01 -0400
committerLinus Torvalds <torvalds@linux-foundation.org>2018-04-11 13:28:38 -0400
commit4ed28639519c7bad5f518e70b3284c6e0763e650 (patch)
tree546506217dea4a0e372ff5080fdbdcc5b569c3c0 /fs
parenta4ff8e8620d3f4f50ac4b41e8067b7d395056843 (diff)
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 eab09532d400: "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 (a87938b2e246: "fs/binfmt_elf.c: fix bug in loading of PIE binaries"), ELF_ET_DYN_BASE moved moved much further from the stack (eab09532d400 and later by c715b72c1ba4: "mm: revert x86_64 and arm64 ELF_ET_DYN_BASE base changes") and excessive stack consumption early during execve fully stopped by da029c11e6b1 ("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. [mhocko@suse.com: fix build] [akpm@linux-foundation.org: coding-style fixes] [avagin@openvz.org: don't use the same value for MAP_FIXED_NOREPLACE and MAP_SYNC] Link: http://lkml.kernel.org/r/20171218184916.24445-1-avagin@openvz.org Link: http://lkml.kernel.org/r/20171213092550.2774-3-mhocko@kernel.org Signed-off-by: Michal Hocko <mhocko@suse.com> Signed-off-by: Andrei Vagin <avagin@openvz.org> Signed-off-by: Michal Hocko <mhocko@suse.com> Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com> Acked-by: Michael Ellerman <mpe@ellerman.id.au> Acked-by: Kees Cook <keescook@chromium.org> Cc: Abdul Haleem <abdhalee@linux.vnet.ibm.com> Cc: Joel Stanley <joel@jms.id.au> Cc: Anshuman Khandual <khandual@linux.vnet.ibm.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'fs')
-rw-r--r--fs/binfmt_elf.c13
1 files changed, 9 insertions, 4 deletions
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 3edca6cb9a33..46f0438088d3 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -377,6 +377,11 @@ static unsigned long elf_map(struct file *filep, unsigned long addr,
377 } else 377 } else
378 map_addr = vm_mmap(filep, addr, size, prot, type, off); 378 map_addr = vm_mmap(filep, addr, size, prot, type, off);
379 379
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
380 return(map_addr); 385 return(map_addr);
381} 386}
382 387
@@ -575,7 +580,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
575 elf_prot |= PROT_EXEC; 580 elf_prot |= PROT_EXEC;
576 vaddr = eppnt->p_vaddr; 581 vaddr = eppnt->p_vaddr;
577 if (interp_elf_ex->e_type == ET_EXEC || load_addr_set) 582 if (interp_elf_ex->e_type == ET_EXEC || load_addr_set)
578 elf_type |= MAP_FIXED; 583 elf_type |= MAP_FIXED_NOREPLACE;
579 else if (no_base && interp_elf_ex->e_type == ET_DYN) 584 else if (no_base && interp_elf_ex->e_type == ET_DYN)
580 load_addr = -vaddr; 585 load_addr = -vaddr;
581 586
@@ -939,7 +944,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
939 * the ET_DYN load_addr calculations, proceed normally. 944 * the ET_DYN load_addr calculations, proceed normally.
940 */ 945 */
941 if (loc->elf_ex.e_type == ET_EXEC || load_addr_set) { 946 if (loc->elf_ex.e_type == ET_EXEC || load_addr_set) {
942 elf_flags |= MAP_FIXED; 947 elf_flags |= MAP_FIXED_NOREPLACE;
943 } else if (loc->elf_ex.e_type == ET_DYN) { 948 } else if (loc->elf_ex.e_type == ET_DYN) {
944 /* 949 /*
945 * This logic is run once for the first LOAD Program 950 * This logic is run once for the first LOAD Program
@@ -975,7 +980,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
975 load_bias = ELF_ET_DYN_BASE; 980 load_bias = ELF_ET_DYN_BASE;
976 if (current->flags & PF_RANDOMIZE) 981 if (current->flags & PF_RANDOMIZE)
977 load_bias += arch_mmap_rnd(); 982 load_bias += arch_mmap_rnd();
978 elf_flags |= MAP_FIXED; 983 elf_flags |= MAP_FIXED_NOREPLACE;
979 } else 984 } else
980 load_bias = 0; 985 load_bias = 0;
981 986
@@ -1235,7 +1240,7 @@ static int load_elf_library(struct file *file)
1235 (eppnt->p_filesz + 1240 (eppnt->p_filesz +
1236 ELF_PAGEOFFSET(eppnt->p_vaddr)), 1241 ELF_PAGEOFFSET(eppnt->p_vaddr)),
1237 PROT_READ | PROT_WRITE | PROT_EXEC, 1242 PROT_READ | PROT_WRITE | PROT_EXEC,
1238 MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE, 1243 MAP_FIXED_NOREPLACE | MAP_PRIVATE | MAP_DENYWRITE,
1239 (eppnt->p_offset - 1244 (eppnt->p_offset -
1240 ELF_PAGEOFFSET(eppnt->p_vaddr))); 1245 ELF_PAGEOFFSET(eppnt->p_vaddr)));
1241 if (error != ELF_PAGESTART(eppnt->p_vaddr)) 1246 if (error != ELF_PAGESTART(eppnt->p_vaddr))