Re: patch: VFS: fix passing of AT_PHDR value in auxv to ELF interpreter

From: Quentin Godfroy
Date: Mon May 07 2007 - 13:48:16 EST

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/