| From 6590725e6fa0eb04edb12c44b6328fe5801ad302 Mon Sep 17 00:00:00 2001 |
| From: Linus Torvalds <torvalds@linux-foundation.org> |
| Date: Sun, 6 Oct 2019 13:53:27 -0700 |
| Subject: [PATCH] elf: don't use MAP_FIXED_NOREPLACE for elf executable |
| mappings |
| |
| commit b212921b13bda088a004328457c5c21458262fe2 upstream. |
| |
| In commit 4ed28639519c ("fs, elf: drop MAP_FIXED usage from elf_map") we |
| changed elf to use MAP_FIXED_NOREPLACE instead of MAP_FIXED for the |
| executable mappings. |
| |
| Then, people reported that it broke some binaries that had overlapping |
| segments from the same file, and commit ad55eac74f20 ("elf: enforce |
| MAP_FIXED on overlaying elf segments") re-instated MAP_FIXED for some |
| overlaying elf segment cases. But only some - despite the summary line |
| of that commit, it only did it when it also does a temporary brk vma for |
| one obvious overlapping case. |
| |
| Now Russell King reports another overlapping case with old 32-bit x86 |
| binaries, which doesn't trigger that limited case. End result: we had |
| better just drop MAP_FIXED_NOREPLACE entirely, and go back to MAP_FIXED. |
| |
| Yes, it's a sign of old binaries generated with old tool-chains, but we |
| do pride ourselves on not breaking existing setups. |
| |
| This still leaves MAP_FIXED_NOREPLACE in place for the load_elf_interp() |
| and the old load_elf_library() use-cases, because nobody has reported |
| breakage for those. Yet. |
| |
| Note that in all the cases seen so far, the overlapping elf sections |
| seem to be just re-mapping of the same executable with different section |
| attributes. We could possibly introduce a new MAP_FIXED_NOFILECHANGE |
| flag or similar, which acts like NOREPLACE, but allows just remapping |
| the same executable file using different protection flags. |
| |
| It's not clear that would make a huge difference to anything, but if |
| people really hate that "elf remaps over previous maps" behavior, maybe |
| at least a more limited form of remapping would alleviate some concerns. |
| |
| Alternatively, we should take a look at our elf_map() logic to see if we |
| end up not mapping things properly the first time. |
| |
| In the meantime, this is the minimal "don't do that then" patch while |
| people hopefully think about it more. |
| |
| Reported-by: Russell King <linux@armlinux.org.uk> |
| Fixes: 4ed28639519c ("fs, elf: drop MAP_FIXED usage from elf_map") |
| Fixes: ad55eac74f20 ("elf: enforce MAP_FIXED on overlaying elf segments") |
| Cc: Michal Hocko <mhocko@suse.com> |
| Cc: Kees Cook <keescook@chromium.org> |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c |
| index 36d172ccb085..1f95ed1ad2ee 100644 |
| --- a/fs/binfmt_elf.c |
| +++ b/fs/binfmt_elf.c |
| @@ -899,7 +899,7 @@ static int load_elf_binary(struct linux_binprm *bprm) |
| the correct location in memory. */ |
| for(i = 0, elf_ppnt = elf_phdata; |
| i < loc->elf_ex.e_phnum; i++, elf_ppnt++) { |
| - int elf_prot, elf_flags, elf_fixed = MAP_FIXED_NOREPLACE; |
| + int elf_prot, elf_flags; |
| unsigned long k, vaddr; |
| unsigned long total_size = 0; |
| |
| @@ -931,13 +931,6 @@ static int load_elf_binary(struct linux_binprm *bprm) |
| */ |
| } |
| } |
| - |
| - /* |
| - * Some binaries have overlapping elf segments and then |
| - * we have to forcefully map over an existing mapping |
| - * e.g. over this newly established brk mapping. |
| - */ |
| - elf_fixed = MAP_FIXED; |
| } |
| |
| elf_prot = make_prot(elf_ppnt->p_flags); |
| @@ -950,7 +943,7 @@ static int load_elf_binary(struct linux_binprm *bprm) |
| * the ET_DYN load_addr calculations, proceed normally. |
| */ |
| if (loc->elf_ex.e_type == ET_EXEC || load_addr_set) { |
| - elf_flags |= elf_fixed; |
| + elf_flags |= MAP_FIXED; |
| } else if (loc->elf_ex.e_type == ET_DYN) { |
| /* |
| * This logic is run once for the first LOAD Program |
| @@ -986,7 +979,7 @@ static int load_elf_binary(struct linux_binprm *bprm) |
| load_bias = ELF_ET_DYN_BASE; |
| if (current->flags & PF_RANDOMIZE) |
| load_bias += arch_mmap_rnd(); |
| - elf_flags |= elf_fixed; |
| + elf_flags |= MAP_FIXED; |
| } else |
| load_bias = 0; |
| |
| -- |
| 2.7.4 |
| |