> On Mar 5, 2018, at 8:26 AM, Peter Zijlstra <pet...@infradead.org> wrote: > > On Mon, Feb 26, 2018 at 09:49:22AM -0800, Song Liu wrote: > >> +/* Parse build ID of ELF file mapped to vma */ >> +static int stack_map_get_build_id(struct vm_area_struct *vma, >> + unsigned char *build_id) >> +{ >> + Elf32_Ehdr *ehdr = (Elf32_Ehdr *)vma->vm_start; > > How does this work? Isn't vma->vm_start a _user_ address? > >> + >> + /* we need at least 1 file header and 1 program header */ >> + if (vma->vm_end - vma->vm_start < >> + sizeof(Elf32_Ehdr) + sizeof(Elf32_Phdr)) >> + return -EINVAL; >> + >> + /* compare magic x7f "ELF" */ >> + if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG) != 0) >> + return -EINVAL; > > So how come you can just dereference it here, without > get_user()/copy_from_user() etc.. ? > >> + >> + /* only support executable file and shared object file */ >> + if (ehdr->e_type != ET_EXEC && ehdr->e_type != ET_DYN) >> + return -EINVAL; >> + >> + if (ehdr->e_ident[EI_CLASS] == 1) >> + return stack_map_get_build_id_32(vma, build_id); >> + else if (ehdr->e_ident[EI_CLASS] == 2) >> + return stack_map_get_build_id_64(vma, build_id); >> + return -EINVAL; >> +}
I will fix this in v2. Thanks! Song