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: <20171004151238.GG24886@x1>
Date:   Wed, 4 Oct 2017 23:12:38 +0800
From:   Baoquan He <bhe@...hat.com>
To:     Michal Hocko <mhocko@...ne.org>
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

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);
+		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