From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751526AbdJDHvD (ORCPT ); Wed, 4 Oct 2017 03:51:03 -0400 Received: from mx2.suse.de ([195.135.220.15]:52604 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751297AbdJDHvC (ORCPT ); Wed, 4 Oct 2017 03:51:02 -0400 Date: Wed, 4 Oct 2017 09:50:59 +0200 From: Michal Hocko To: Linus Torvalds , Kees Cook , Oleg Nesterov , Jiri Kosina , Al Viro , Ingo Molnar , Baoquan He Cc: LKML Subject: MAP_FIXED for ELF mappings Message-ID: <20171004075059.bbx7madwgwflb7ky@dhcp22.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, while studying CVE-2017-1000253 and the MAP_FIXED usage in load_elf* code paths I have stumbled over MAP_FIXED usage for elf segments mapping. I am not really familiar with this area much so I might draw completely incorrect conclusions here but I am really wondering why we are doing MAP_FIXED there at all. I can see why some segments really have to be mapped at a specific address but I wonder whether MAP_FIXED is the right tool to achieve that. It seems to me that MAP_FIXED is fundamentally dangerous because it unmaps any existing mapping. I assume that nothing should be really mapped in the requested range that early so we can only stumble over something when the address space randomization place things unexpectedly (which was the case of the above mentioned CVE AFAIU). So my primary question is whether we can/should simply drop MAP_FIXED from elf_map at all. Instead we should test whether the mapping was successful for the requested address and fail otherwise. I realize that failing due to something that a user has no idea about sucks a lot but it seems to me safer to simply complain into the log and fail is a safer option. Something like the following completely untested diff. Or am I completely missing the point of the MAP_FIXED purpose? --- diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 6466153f2bf0..09456e2add18 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -341,6 +341,29 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec, #ifndef elf_map +static unsigned long elf_vm_mmap(struct file *filep, unsigned long addr, + unsigned long size, int prot, int type, unsigned long off) +{ + unsigned long map_addr; + + /* + * If caller requests the mapping at a specific place, make sure we fail + * rather than potentially clobber an existing mapping which can have + * security consequences (e.g. smash over the stack area). + */ + map_addr = vm_mmap(filep, addr, size, prot, type & ~MAP_FIXED, off); + if (BAD_ADDR(map_addr)) + return map_addr; + + if ((type & MAP_FIXED) && map_addr != addr) { + pr_info("Uhuuh, elf segement at %p requested but the memory is mapped already\n", + (void*)addr); + return -EAGAIN; + } + + return map_addr; +} + static unsigned long elf_map(struct file *filep, unsigned long addr, struct elf_phdr *eppnt, int prot, int type, unsigned long total_size) @@ -366,11 +389,11 @@ static unsigned long elf_map(struct file *filep, unsigned long addr, */ if (total_size) { total_size = ELF_PAGEALIGN(total_size); - map_addr = vm_mmap(filep, addr, total_size, prot, type, off); + map_addr = elf_vm_mmap(filep, addr, total_size, prot, type, off); if (!BAD_ADDR(map_addr)) vm_munmap(map_addr+size, total_size-size); } else - map_addr = vm_mmap(filep, addr, size, prot, type, off); + map_addr = elf_vm_mmap(filep, addr, size, prot, type, off); return(map_addr); } @@ -1215,7 +1238,7 @@ static int load_elf_library(struct file *file) eppnt++; /* Now use mmap to map the library into memory. */ - error = vm_mmap(file, + error = elf_vm_mmap(file, ELF_PAGESTART(eppnt->p_vaddr), (eppnt->p_filesz + ELF_PAGEOFFSET(eppnt->p_vaddr)), -- Michal Hocko SUSE Labs