Re: MAP_FIXED for ELF mappings

From: Michal Hocko <mhocko@kernel.org>
To: Baoquan He <bhe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Kees Cook <keescook@chromium.org>,
	Oleg Nesterov <oleg@redhat.com>, Jiri Kosina <jkosina@suse.cz>,
	Al Viro <viro@zeniv.linux.org.uk>, Ingo Molnar <mingo@elte.hu>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: MAP_FIXED for ELF mappings
Date: Wed, 4 Oct 2017 17:11:14 +0200	[thread overview]
Message-ID: <20171004151114.7gwppd5qh242laor@dhcp22.suse.cz> (raw)
In-Reply-To: <20171004150334.GB31992@x1>

On Wed 04-10-17 23:03:34, Baoquan He wrote:
> On 10/04/17 at 09:50am, Michal Hocko wrote:
> > 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.
> 
> Sorry to interrupt. I tried below example.c and example2.c files and
> compile and link them with fpie and pie. Seems in the final PIE
> executable, the local global is pc relative. Means the data segment has
> to be put after the code segment though PIE program. Then MAP_FIXED is a
> good to have flag, especially we have counted in the total_size to
> search an area to cover the whole dynamic program and get the load_bias.
> It's no way to fail to get map agrea in case load_addr_set == 1.
> So with MAP_FIXED set, we won't take time to search the mm vma rb
> tree when load_addr_set == 1, but just return the specified addr directly,
> looks more efficient.

I am sorry, but I do not follow. elf_map should get the address hint to
use. MAP_FIXED merely unmaps anything underneath if there is something
which is exactly what I called dangerous. Without the flag we would just
fail in that case. Or, are you suggesting that MAP_FIXED is a
performance optimization because we are not doing find_vma in that case?
-- 
Michal Hocko
SUSE Labs