[PATCH] Races in irixelf.c - same type as binfmt_aout.c stuff.

Alexander Viro (viro@math.psu.edu)
Sat, 26 Jun 1999 19:13:59 -0400 (EDT)

do_load_irix_binary() doesn't fget() the binary it loads. As the
result we have a race with close() leaving a dangling pointers. Ditto for
do_load_irix_library(). The following patch fixes it (it's the same
problem as in do_load_aout_binary() and was obviously copied from there.
When the copy in binfmt_elf.c was fixed (2.1.89) nobody cared to propagate
the fix... Wonders of code duplication ;-/) Please, apply it.
TIA,
Al

diff -urN linux-2.3.9-pre5/arch/mips/kernel/irixelf.c linux-bird.smp/arch/mips/kernel/irixelf.c
--- linux-2.3.9-pre5/arch/mips/kernel/irixelf.c Sat Jun 26 16:39:11 1999
+++ linux-bird.smp/arch/mips/kernel/irixelf.c Sat Jun 26 18:51:19 1999
@@ -308,7 +308,7 @@
return 0xffffffff;
}

- file = current->files->fd[elf_exec_fileno];
+ file = fget(elf_exec_fileno);

eppnt = elf_phdata;
for(i=0; i<interp_elf_ex->e_phnum; i++, eppnt++) {
@@ -366,6 +366,7 @@
}

/* Now use mmap to map the library into memory. */
+ fput(file);
sys_close(elf_exec_fileno);
if(error < 0 && error > -1024) {
#ifdef DEBUG_ELF
@@ -615,6 +616,7 @@
unsigned int start_code, end_code, end_data, elf_stack;
int elf_exec_fileno, retval, has_interp, has_ephdr, size, i;
char *elf_interpreter;
+ struct file *file;
mm_segment_t old_fs;

load_addr = 0;
@@ -637,10 +639,8 @@

retval = read_exec(bprm->dentry, elf_ex.e_phoff,
(char *) elf_phdata, size, 1);
- if (retval < 0) {
- kfree (elf_phdata);
- return retval;
- }
+ if (retval < 0)
+ goto out_phdata;

#ifdef DEBUG_ELF
dump_phdrs(elf_phdata, elf_ex.e_phnum);
@@ -665,12 +665,10 @@

elf_bss = 0;
elf_brk = 0;
- elf_exec_fileno = open_dentry(bprm->dentry, O_RDONLY);
-
- if (elf_exec_fileno < 0) {
- kfree (elf_phdata);
- return elf_exec_fileno;
- }
+ retval = open_dentry(bprm->dentry, O_RDONLY);
+ if (retval < 0)
+ goto out_phdata;
+ file = fget(elf_exec_fileno = retval);

elf_stack = 0xffffffff;
elf_interpreter = NULL;
@@ -682,40 +680,26 @@
&interpreter_dentry,
&interp_elf_ex, elf_phdata, bprm,
elf_ex.e_phnum);
- if(retval) {
- kfree(elf_phdata);
- sys_close(elf_exec_fileno);
- return retval;
- }
+ if(retval)
+ goto out_file;

if(elf_interpreter) {
retval = verify_irix_interpreter(&interp_elf_ex);
- if(retval) {
- kfree(elf_interpreter);
- kfree(elf_phdata);
- sys_close(elf_exec_fileno);
- return retval;
- }
+ if(retval)
+ goto out_interp;
}

/* OK, we are done with that, now set up the arg stuff,
* and then start this sucker up.
*/
- if (!bprm->sh_bang) {
- if (!bprm->p) {
- if(elf_interpreter) {
- kfree(elf_interpreter);
- }
- kfree (elf_phdata);
- sys_close(elf_exec_fileno);
- return -E2BIG;
- }
- }
+ retval = -E2BIG;
+ if (!bprm->sh_bang && !bprm->p)
+ goto out_interp;

/* Flush all traces of the currently running executable */
retval = flush_old_exec(bprm);
if (retval)
- return retval;
+ goto out_interp;

/* OK, This is the point of no return */
current->mm->end_data = 0;
@@ -737,8 +721,7 @@
old_fs = get_fs();
set_fs(get_ds());

- map_executable(current->files->fd[elf_exec_fileno], elf_phdata,
- elf_ex.e_phnum, &elf_stack, &load_addr,
+ map_executable(file, elf_phdata, elf_ex.e_phnum, &elf_stack, &load_addr,
&start_code, &elf_bss, &end_code, &end_data, &elf_brk);

if(elf_interpreter) {
@@ -758,6 +741,7 @@
set_fs(old_fs);

kfree(elf_phdata);
+ fput(file);
sys_close(elf_exec_fileno);
current->personality = PER_IRIX32;

@@ -820,6 +804,17 @@
if (current->flags & PF_PTRACED)
send_sig(SIGTRAP, current, 0);
return 0;
+
+out_interp:
+ if(elf_interpreter) {
+ kfree(elf_interpreter);
+ }
+out_file:
+ fput(file);
+ sys_close(elf_exec_fileno);
+out_phdata:
+ kfree (elf_phdata);
+ return retval;
}

static int load_irix_binary(struct linux_binprm * bprm, struct pt_regs * regs)
@@ -835,9 +830,8 @@
/* This is really simpleminded and specialized - we are loading an
* a.out library that is given an ELF header.
*/
-static inline int do_load_irix_library(int fd)
+static inline int do_load_irix_library(struct file *file)
{
- struct file * file;
struct elfhdr elf_ex;
struct elf_phdr *elf_phdata = NULL;
struct dentry *dentry;
@@ -850,14 +844,12 @@
int i,j, k;

len = 0;
- file = current->files->fd[fd];
+ if (!file->f_op)
+ return -EACCES;
dentry = file->f_dentry;
inode = dentry->d_inode;
elf_bss = 0;

- if (!file || !file->f_op)
- return -EACCES;
-
/* Seek to the beginning of the file. */
if (file->f_op->llseek) {
if ((error = file->f_op->llseek(file, 0, 0)) != 0)
@@ -934,10 +926,15 @@

static int load_irix_library(int fd)
{
- int retval;
+ int retval = -EACCES;
+ struct file *file;

MOD_INC_USE_COUNT;
- retval = do_load_irix_library(fd);
+ file = fget(fd);
+ if (file) {
+ retval = do_load_irix_library(file);
+ fput(file);
+ }
MOD_DEC_USE_COUNT;
return retval;
}
@@ -978,9 +975,12 @@
return -ENOEXEC;
}

- filp = current->files->fd[fd];
- if(!filp || !filp->f_op) {
+ filp = fget(fd);
+ if (!filp)
+ return -EACCES;
+ if(!filp->f_op) {
printk("irix_mapelf: Bogon filp!\n");
+ fput(file);
return -EACCES;
}

@@ -998,6 +998,7 @@

if(retval != (hp->p_vaddr & 0xfffff000)) {
printk("irix_mapelf: do_mmap fails with %d!\n", retval);
+ fput(file);
return retval;
}
}
@@ -1005,6 +1006,7 @@
#ifdef DEBUG_ELF
printk("irix_mapelf: Success, returning %08lx\n", user_phdrp->p_vaddr);
#endif
+ fput(file);
return user_phdrp->p_vaddr;
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/