Messages in this thread Patch in this message | | | Date | Sat, 26 Jun 1999 19:13:59 -0400 (EDT) | From | Alexander Viro <> | Subject | [PATCH] Races in irixelf.c - same type as binfmt_aout.c stuff. |
| |
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/
| |