linux-kernel - Re: MAP_FIXED for ELF mappings

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20171004151700.k4fvivvmi7pm5sl7@dhcp22.suse.cz>
Date:   Wed, 4 Oct 2017 17:17:00 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     Baoquan He <bhe@...hat.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>,
        Oleg Nesterov <oleg@...hat.com>, Jiri Kosina <jkosina@...e.cz>,
        Al Viro <viro@...iv.linux.org.uk>, Ingo Molnar <mingo@...e.hu>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: MAP_FIXED for ELF mappings

On Wed 04-10-17 23:12:38, Baoquan He wrote:
> I made a clean up patch according to Oleg's suggestion. It's trying to
> get an map area to cover total_size, then do mmap for for the 1st
> program segment only. Not sure if this way is correct.
> 
> >From 40f231bb78a74caebcb4a898089a9fa5323be05f Mon Sep 17 00:00:00 2001
> From: Baoquan He <bhe@...hat.com>
> Date: Fri, 29 Sep 2017 21:35:30 +0800
> Subject: [PATCH] binfmt_elf: Clean up the elf_map
> 
> Oleg pointed out that it's really ugly to do mmap of the total_size, then
> unmap the region excluding the 1st segment. The right way should be search
> an unmapped area which can cover region of total_size, then map the 1st
> segment only.
> 
> And also update the code comment accordingly. In below commit, the relevant
> code comment is not changed to cover the ELF binary image case.
> commit a87938b2e2 ("fs/binfmt_elf.c: fix bug in loading of PIE binaries")
> 
> Signed-off-by: Baoquan He <bhe@...hat.com>
> ---
>  fs/binfmt_elf.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 72b7ecba7ead..43a47b2aa3f6 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -357,22 +357,25 @@ static unsigned long elf_map(struct file *filep, unsigned long addr,
>  		return addr;
>  
>  	/*
> -	* total_size is the size of the ELF (interpreter) image.
> -	* The _first_ mmap needs to know the full size, otherwise
> -	* randomization might put this image into an overlapping
> -	* position with the ELF binary image. (since size < total_size)
> -	* So we first map the 'big' image - and unmap the remainder at
> -	* the end. (which unmap is needed for ELF images with holes.)
> +	* total_size is the size of the ELF binary image or the ELF loader
> +	* image. For loader image, the _first_ mmap needs to know the full
> +	* size, otherwise randomization might put image into an overlapping
> +	* position with the ELF binary image.(since size < total_size)
> +	* So we use total_size to get an area to cover the whole loader image,
> +	* then map the 1st progment segment only with its own size. For binary
> +	* image, similarly, the _first_ mmap also needs to know the full size,
> +	* otherwise randomization might put image above mm->mmap_base.
>  	*/
>  	if (total_size) {
>  		total_size = ELF_PAGEALIGN(total_size);
> -		map_addr = vm_mmap(filep, addr, total_size, prot, flags, off);
> -		if (!BAD_ADDR(map_addr))
> -			vm_munmap(map_addr+size, total_size-size);
> -	} else
> -		map_addr = vm_mmap(filep, addr, size, prot, flags, off);
> +		addr = get_unmapped_area(file, addr, total_size, off, flags);

So how does this prevent clobbering an existing VMA when flags contains
MAP_FIXED?

> +		if (offset_in_page(addr))
> +			return addr;
> +	}
> +
> +	map_addr = vm_mmap(filep, addr, size, prot, flags, off);
>  
> -	return(map_addr);
> +	return (map_addr);
>  }
>  
>  #endif /* !elf_map */
> -- 
> 2.5.5

-- 
Michal Hocko
SUSE Labs