On 2020/1/31 14:16, Andrew Morton wrote: > From: Alexey Dobriyan <adobriyan@xxxxxxxxx> > Subject: fs/binfmt_elf.c: don't copy ELF header around > > ELF header is read into bprm->buf[] by generic execve code. > > Save a memcpy and allocate just one header for the interpreter instead of > two headers (64 bytes instead of 128 on 64-bit). > > Link: http://lkml.kernel.org/r/20191208171242.GA19716@avx2 > Signed-off-by: Alexey Dobriyan <adobriyan@xxxxxxxxx> > Reviewed-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > --- > > fs/binfmt_elf.c | 55 ++++++++++++++++++++++------------------------ > 1 file changed, 27 insertions(+), 28 deletions(-) > > --- a/fs/binfmt_elf.c~elf-dont-copy-elf-header-around > +++ a/fs/binfmt_elf.c > @@ -161,8 +161,9 @@ static int padzero(unsigned long elf_bss > #endif > > static int > -create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec, > - unsigned long load_addr, unsigned long interp_load_addr) > +create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec, > + unsigned long load_addr, unsigned long interp_load_addr, > + unsigned long e_entry) > { > unsigned long p = bprm->p; > int argc = bprm->argc; > @@ -251,7 +252,7 @@ create_elf_tables(struct linux_binprm *b > NEW_AUX_ENT(AT_PHNUM, exec->e_phnum); > NEW_AUX_ENT(AT_BASE, interp_load_addr); > NEW_AUX_ENT(AT_FLAGS, 0); > - NEW_AUX_ENT(AT_ENTRY, exec->e_entry); > + NEW_AUX_ENT(AT_ENTRY, e_entry); > NEW_AUX_ENT(AT_UID, from_kuid_munged(cred->user_ns, cred->uid)); > NEW_AUX_ENT(AT_EUID, from_kuid_munged(cred->user_ns, cred->euid)); > NEW_AUX_ENT(AT_GID, from_kgid_munged(cred->user_ns, cred->gid)); > @@ -690,12 +691,13 @@ static int load_elf_binary(struct linux_ > int bss_prot = 0; > int retval, i; > unsigned long elf_entry; > + unsigned long e_entry; > unsigned long interp_load_addr = 0; > unsigned long start_code, end_code, start_data, end_data; > unsigned long reloc_func_desc __maybe_unused = 0; > int executable_stack = EXSTACK_DEFAULT; > + struct elfhdr *elf_ex = (struct elfhdr *)bprm->buf; > struct { > - struct elfhdr elf_ex; > struct elfhdr interp_elf_ex; > } *loc; > struct arch_elf_state arch_state = INIT_ARCH_ELF_STATE; > @@ -706,30 +708,27 @@ static int load_elf_binary(struct linux_ > retval = -ENOMEM; > goto out_ret; > } > - > - /* Get the exec-header */ > - loc->elf_ex = *((struct elfhdr *)bprm->buf); > > retval = -ENOEXEC; > /* First of all, some simple consistency checks */ > - if (memcmp(loc->elf_ex.e_ident, ELFMAG, SELFMAG) != 0) > + if (memcmp(elf_ex->e_ident, ELFMAG, SELFMAG) != 0) > goto out; > > - if (loc->elf_ex.e_type != ET_EXEC && loc->elf_ex.e_type != ET_DYN) > + if (elf_ex->e_type != ET_EXEC && elf_ex->e_type != ET_DYN) > goto out; > - if (!elf_check_arch(&loc->elf_ex)) > + if (!elf_check_arch(elf_ex)) > goto out; > - if (elf_check_fdpic(&loc->elf_ex)) > + if (elf_check_fdpic(elf_ex)) > goto out; > if (!bprm->file->f_op->mmap) > goto out; > > - elf_phdata = load_elf_phdrs(&loc->elf_ex, bprm->file); > + elf_phdata = load_elf_phdrs(elf_ex, bprm->file); > if (!elf_phdata) > goto out; > > elf_ppnt = elf_phdata; > - for (i = 0; i < loc->elf_ex.e_phnum; i++, elf_ppnt++) { > + for (i = 0; i < elf_ex->e_phnum; i++, elf_ppnt++) { > char *elf_interpreter; > > if (elf_ppnt->p_type != PT_INTERP) > @@ -783,7 +782,7 @@ out_free_interp: > } > > elf_ppnt = elf_phdata; > - for (i = 0; i < loc->elf_ex.e_phnum; i++, elf_ppnt++) > + for (i = 0; i < elf_ex->e_phnum; i++, elf_ppnt++) > switch (elf_ppnt->p_type) { > case PT_GNU_STACK: > if (elf_ppnt->p_flags & PF_X) > @@ -793,7 +792,7 @@ out_free_interp: > break; > > case PT_LOPROC ... PT_HIPROC: > - retval = arch_elf_pt_proc(&loc->elf_ex, elf_ppnt, > + retval = arch_elf_pt_proc(elf_ex, elf_ppnt, > bprm->file, false, > &arch_state); > if (retval) > @@ -837,7 +836,7 @@ out_free_interp: > * still possible to return an error to the code that invoked > * the exec syscall. > */ > - retval = arch_check_elf(&loc->elf_ex, > + retval = arch_check_elf(elf_ex, > !!interpreter, &loc->interp_elf_ex, > &arch_state); > if (retval) > @@ -850,8 +849,8 @@ out_free_interp: > > /* Do this immediately, since STACK_TOP as used in setup_arg_pages > may depend on the personality. */ > - SET_PERSONALITY2(loc->elf_ex, &arch_state); > - if (elf_read_implies_exec(loc->elf_ex, executable_stack)) > + SET_PERSONALITY2(*elf_ex, &arch_state); It seems that the "SET_PERSONALITY2()" is a little late. When a 32-bit compatible user-mode program forks out a 64-bit program, when the 64-bit program is run in execve() the 32-bit STACK_TOP_MAX is used to set vm_end and vm_start of the vma in __bprm_mm_init() in alloc_bprm() because the 32-bit compatible flag has not been cleared, but the setup_arg_pages() function later uses 64-bit STACK_TOP after calling this SET_PERSONALITY2() to clear the 32-bit compatible flag, which doesn't seem reasonable. > + if (elf_read_implies_exec(*elf_ex, executable_stack)) > current->personality |= READ_IMPLIES_EXEC; > > if (!(current->personality & ADDR_NO_RANDOMIZE) && randomize_va_space) > @@ -878,7 +877,7 @@ out_free_interp: > /* Now we do a little grungy work by mmapping the ELF image into > the correct location in memory. */ > for(i = 0, elf_ppnt = elf_phdata; > - i < loc->elf_ex.e_phnum; i++, elf_ppnt++) { > + i < elf_ex->e_phnum; i++, elf_ppnt++) { > int elf_prot, elf_flags; > unsigned long k, vaddr; > unsigned long total_size = 0; > @@ -922,9 +921,9 @@ out_free_interp: > * If we are loading ET_EXEC or we have already performed > * the ET_DYN load_addr calculations, proceed normally. > */ > - if (loc->elf_ex.e_type == ET_EXEC || load_addr_set) { > + if (elf_ex->e_type == ET_EXEC || load_addr_set) { > elf_flags |= MAP_FIXED; > - } else if (loc->elf_ex.e_type == ET_DYN) { > + } else if (elf_ex->e_type == ET_DYN) { > /* > * This logic is run once for the first LOAD Program > * Header for ET_DYN binaries to calculate the > @@ -973,7 +972,7 @@ out_free_interp: > load_bias = ELF_PAGESTART(load_bias - vaddr); > > total_size = total_mapping_size(elf_phdata, > - loc->elf_ex.e_phnum); > + elf_ex->e_phnum); > if (!total_size) { > retval = -EINVAL; > goto out_free_dentry; > @@ -991,7 +990,7 @@ out_free_interp: > if (!load_addr_set) { > load_addr_set = 1; > load_addr = (elf_ppnt->p_vaddr - elf_ppnt->p_offset); > - if (loc->elf_ex.e_type == ET_DYN) { > + if (elf_ex->e_type == ET_DYN) { > load_bias += error - > ELF_PAGESTART(load_bias + vaddr); > load_addr += load_bias; > @@ -1032,7 +1031,7 @@ out_free_interp: > } > } > > - loc->elf_ex.e_entry += load_bias; > + e_entry = elf_ex->e_entry + load_bias; > elf_bss += load_bias; > elf_brk += load_bias; > start_code += load_bias; > @@ -1075,7 +1074,7 @@ out_free_interp: > allow_write_access(interpreter); > fput(interpreter); > } else { > - elf_entry = loc->elf_ex.e_entry; > + elf_entry = e_entry; > if (BAD_ADDR(elf_entry)) { > retval = -EINVAL; > goto out_free_dentry; > @@ -1093,8 +1092,8 @@ out_free_interp: > goto out; > #endif /* ARCH_HAS_SETUP_ADDITIONAL_PAGES */ > > - retval = create_elf_tables(bprm, &loc->elf_ex, > - load_addr, interp_load_addr); > + retval = create_elf_tables(bprm, elf_ex, > + load_addr, interp_load_addr, e_entry); > if (retval < 0) > goto out; > current->mm->end_code = end_code; > @@ -1112,7 +1111,7 @@ out_free_interp: > * growing down), and into the unused ELF_ET_DYN_BASE region. > */ > if (IS_ENABLED(CONFIG_ARCH_HAS_ELF_RANDOMIZE) && > - loc->elf_ex.e_type == ET_DYN && !interpreter) > + elf_ex->e_type == ET_DYN && !interpreter) > current->mm->brk = current->mm->start_brk = > ELF_ET_DYN_BASE; > > _
