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 <[email protected]> > 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 <[email protected]> > --- > 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
