On Tue 17-10-17 20:26:14, Baoquan He wrote:
> 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.
> 
> <[email protected]>: host mail.kerne.org[104.131.33.237] said: 454 4.1.1
>     <[email protected]>: Recipient address rejected: User unknown in virtual
>     mailbox table (in reply to RCPT TO command)

yes, there was a typo in the email address which I've fixed up in the
reply... Your cleanup was mostly about replacing vm_mmap by
get_unmaped_area which is an independent issue to what I am trying to
achieve here.

> On 10/16/17 at 03:44pm, Michal Hocko wrote:
> > From: Michal Hocko <[email protected]>
> > 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.

Could you be more specific please?

> 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.

I've added another level of helper to keep the code in elf_map saner. It
is quite complex already.
-- 
Michal Hocko
SUSE Labs

Reply via email to