[CCing metag people for the metag elf_map implementation specific. The thread starts here http://lkml.kernel.org/r/20171016134446.19910-1-mhocko@xxxxxxxxxx] On Mon 16-10-17 09:39:14, Kees Cook wrote: > On Mon, Oct 16, 2017 at 6:44 AM, Michal Hocko <mhocko@xxxxxxxxxx> wrote: [...] > > +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); > > Is "info" loud enough? I actually think this should be a WARN_ONCE(). Does it make any sense to taint the kernel just because of this condition? I can make it pr_error... > > + 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, > > elf_map is redirected on metag -- it should probably have its vm_mmap > calls adjust too. Thanks for spotting this. I am not really familiar with metag. It seems to clear MAP_FIXED already tcm_tag = tcm_lookup_tag(addr); if (tcm_tag != TCM_INVALID_TAG) type &= ~MAP_FIXED; So if there is a tag the flag is cleared. I do not understand this code (and git log doesn't help) but why is this MAP_FIXED code really needed? > > */ > > 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); > > } else > > - map_addr = vm_mmap(filep, addr, size, prot, type, off); > > + map_addr = elf_vm_mmap(filep, addr, size, prot, type, off); > > > > return(map_addr); > > } > > @@ -1215,7 +1238,7 @@ static int load_elf_library(struct file *file) > > eppnt++; > > > > /* Now use mmap to map the library into memory. */ > > - error = vm_mmap(file, > > + error = elf_vm_mmap(file, > > ELF_PAGESTART(eppnt->p_vaddr), > > (eppnt->p_filesz + > > ELF_PAGEOFFSET(eppnt->p_vaddr)), > > -- > > 2.14.2 > > > > Otherwise, yeah, this should be good. Thanks! -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-metag" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
