On Fri, May 04, 2007 at 10:09:21AM -0400, Quentin Godfroy wrote: > On a dynamic ELF executable, the current kernel loader gives to the > interpreter (in the AUXV vector) the AT_PHDR argument as : > offset_of_phdr_in_file + first address. > > It can be wrong for an executable where the program headers are not located > in the first loaded segment. > > This patch corrects the behaviour. > > Signed-off-by: Quentin Godfroy <godfroy@xxxxxxxxxxxxxx> > --- > Here is an example of such an ELF executable which the current code > fails on : > ftp://quatramaran.ens.fr/pub/godfroy/addrpath/broken-sample > > --- linux-2.6.21.1/fs/binfmt_elf.c 2007-05-04 03:20:00.000000000 -0400 > +++ linux-2.6.21.1-patch/fs/binfmt_elf.c 2007-05-04 08:02:18.000000000 -0400 > @@ -134,6 +134,7 @@ static int padzero(unsigned long elf_bss > static int > create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec, > int interp_aout, unsigned long load_addr, > + unsigned long phdr_addr, > unsigned long interp_load_addr) > { > unsigned long p = bprm->p; > @@ -190,7 +191,7 @@ create_elf_tables(struct linux_binprm *b > NEW_AUX_ENT(AT_HWCAP, ELF_HWCAP); > NEW_AUX_ENT(AT_PAGESZ, ELF_EXEC_PAGESIZE); > NEW_AUX_ENT(AT_CLKTCK, CLOCKS_PER_SEC); > - NEW_AUX_ENT(AT_PHDR, load_addr + exec->e_phoff); > + NEW_AUX_ENT(AT_PHDR, phdr_addr); > NEW_AUX_ENT(AT_PHENT, sizeof(struct elf_phdr)); > NEW_AUX_ENT(AT_PHNUM, exec->e_phnum); > NEW_AUX_ENT(AT_BASE, interp_load_addr); > @@ -529,7 +530,7 @@ static unsigned long randomize_stack_top > static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs) > { > struct file *interpreter = NULL; /* to shut gcc up */ > - unsigned long load_addr = 0, load_bias = 0; > + unsigned long load_addr = 0, load_bias = 0, phdr_addr = 0; > int load_addr_set = 0; > char * elf_interpreter = NULL; > unsigned int interpreter_type = INTERPRETER_NONE; > @@ -718,6 +719,16 @@ static int load_elf_binary(struct linux_ > break; > } > > + elf_ppnt = elf_phdata; > + for (i = 0; i< loc->elf_ex.e_phnum; i++, elf_ppnt++) > + if (elf_ppnt->p_type == PT_PHDR) { > + phdr_addr = elf_ppnt->p_vaddr; > + break; > + } > + retval = -ENOEXEC; > + if (!phdr_addr) > + goto out_free_dentry; > + > /* Some simple consistency checks for the interpreter */ > if (elf_interpreter) { > interpreter_type = INTERPRETER_ELF | INTERPRETER_AOUT; > @@ -987,7 +998,7 @@ static int load_elf_binary(struct linux_ > current->flags &= ~PF_FORKNOEXEC; > create_elf_tables(bprm, &loc->elf_ex, > (interpreter_type == INTERPRETER_AOUT), > - load_addr, interp_load_addr); > + load_addr, phdr_addr, interp_load_addr); > /* N.B. passed_fileno might not be initialized? */ > if (interpreter_type == INTERPRETER_AOUT) > current->mm->arg_start += strlen(passed_fileno) + 1; I have made another patch, which I hope should not break anything this time. I tested it on an x86_64 kernel with 32 and 64 bits executables, PIE and not PIE (well my only PIE are libc-2.5.so and ld-2.5.so). I rejoined the loops as suggested, and the kernel falls back on load_addr + exec->e_phoff if there was no PT_PHDR entry in the program headers. Signed-off-by: Quentin Godfroy <godfroy@xxxxxxxxxxxxxx> --- --- linux-2.6.21.1/fs/binfmt_elf.c 2007-05-07 10:58:38.000000000 -0400 +++ linux-2.6.21.1-patch/fs/binfmt_elf.c 2007-05-07 13:14:33.000000000 -0400 @@ -134,6 +134,7 @@ static int padzero(unsigned long elf_bss static int create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec, int interp_aout, unsigned long load_addr, + unsigned long phdr_addr, unsigned long interp_load_addr) { unsigned long p = bprm->p; @@ -190,7 +191,10 @@ create_elf_tables(struct linux_binprm *b NEW_AUX_ENT(AT_HWCAP, ELF_HWCAP); NEW_AUX_ENT(AT_PAGESZ, ELF_EXEC_PAGESIZE); NEW_AUX_ENT(AT_CLKTCK, CLOCKS_PER_SEC); - NEW_AUX_ENT(AT_PHDR, load_addr + exec->e_phoff); + if(phdr_addr) + NEW_AUX_ENT(AT_PHDR, phdr_addr); + else + NEW_AUX_ENT(AT_PHDR, load_addr + exec->e_phoff); NEW_AUX_ENT(AT_PHENT, sizeof(struct elf_phdr)); NEW_AUX_ENT(AT_PHNUM, exec->e_phnum); NEW_AUX_ENT(AT_BASE, interp_load_addr); @@ -529,7 +533,7 @@ static unsigned long randomize_stack_top static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs) { struct file *interpreter = NULL; /* to shut gcc up */ - unsigned long load_addr = 0, load_bias = 0; + unsigned long load_addr = 0, load_bias = 0, phdr_addr = 0; int load_addr_set = 0; char * elf_interpreter = NULL; unsigned int interpreter_type = INTERPRETER_NONE; @@ -620,7 +624,12 @@ static int load_elf_binary(struct linux_ start_data = 0; end_data = 0; - for (i = 0; i < loc->elf_ex.e_phnum; i++) { + for (i = 0; i < loc->elf_ex.e_phnum; i++, elf_ppnt++) { + if (elf_ppnt->p_type == PT_PHDR) { + phdr_addr = elf_ppnt->p_vaddr; + continue; + } + if (elf_ppnt->p_type == PT_INTERP) { /* This is the program interpreter used for * shared libraries - for now assume that this @@ -703,20 +712,17 @@ static int load_elf_binary(struct linux_ /* Get the exec headers */ loc->interp_ex = *((struct exec *)bprm->buf); loc->interp_elf_ex = *((struct elfhdr *)bprm->buf); - break; + continue; } - elf_ppnt++; - } - elf_ppnt = elf_phdata; - for (i = 0; i < loc->elf_ex.e_phnum; i++, elf_ppnt++) if (elf_ppnt->p_type == PT_GNU_STACK) { if (elf_ppnt->p_flags & PF_X) executable_stack = EXSTACK_ENABLE_X; else executable_stack = EXSTACK_DISABLE_X; - break; + continue; } + } /* Some simple consistency checks for the interpreter */ if (elf_interpreter) { @@ -985,9 +991,12 @@ static int load_elf_binary(struct linux_ compute_creds(bprm); current->flags &= ~PF_FORKNOEXEC; + if (phdr_addr) + phdr_addr += load_bias; + create_elf_tables(bprm, &loc->elf_ex, (interpreter_type == INTERPRETER_AOUT), - load_addr, interp_load_addr); + load_addr, phdr_addr, interp_load_addr); /* N.B. passed_fileno might not be initialized? */ if (interpreter_type == INTERPRETER_AOUT) current->mm->arg_start += strlen(passed_fileno) + 1; - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
