linux-kernel - Re: [PATCH 1/2] fs, elf: drop MAP_FIXED usage from elf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 17 Oct 2017 20:26:14 +0800
From:   Baoquan He <bhe@...hat.com>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>,
        Jiri Kosina <jkosina@...e.cz>,
        Al Viro <viro@...iv.linux.org.uk>,
        Oleg Nesterov <oleg@...hat.com>,
        Ingo Molnar <mingo@...hat.com>, Michal Hocko <mhocko@...e.com>
Subject: Re: [PATCH 1/2] fs, elf: drop MAP_FIXED usage from elf_map

Hi Michal,

Earlier I posted a patchset to clean up these in a different way, but
patches sent to your below mail address are all rejected.

<mhocko@...ne.org>: host mail.kerne.org[104.131.33.237] said: 454 4.1.1
    <mhocko@...ne.org>: Recipient address rejected: User unknown in virtual
    mailbox table (in reply to RCPT TO command)

On 10/16/17 at 03:44pm, Michal Hocko wrote:
> From: Michal Hocko <mhocko@...e.com>
> 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);

Here we will still take the map total, then unmap the rest way.

I am wondering why we don't fix those issues we figured out, but add 
another level of wrapper elf_vm_mmap() to hack it. So we will have
elf_map() -> elf_vm_mmap() -> vm_mmap(), not even counting
vm_mmap_pgoff(), then finally enter into do_mmap_pgoff(), to do the
maping for elf program.

Thanks
Baoquan


>  	} 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)),
> -- 
> 2.14.2
>