Solve the problem where it is possible to get the kernel stuck in
a loop down in pmap_init_pt(). A subtraction causes the number of pages to become negative, that was assigned to an unsigned variable, and there is a lot of iteration. The bug is due to the ELF image activator not properly checking for its files being the correct size as specified by the ELF header. The solution is to check that the header doesn't ask for part of a file when that part of the file doesn't exist. Make sure to set VEXEC at the proper times to make the executables immutable (remove race conditions). Also, the ELF format specifiies header entries that allow embedding of other executables (hence how ld-elf.so.1 gets loaded, but not the same as loading shared libraries), so those executables need to be set VEXEC, too, so they're immutable. Reviewed by: peter
This commit is contained in:
parent
11c5444470
commit
25ead03462
@ -61,6 +61,7 @@
|
||||
#include <vm/vm_object.h>
|
||||
#include <vm/vm_extern.h>
|
||||
|
||||
#include <machine/atomic.h>
|
||||
#include <machine/elf.h>
|
||||
#include <machine/md_var.h>
|
||||
|
||||
@ -190,6 +191,21 @@ elf_load_section(struct proc *p, struct vmspace *vmspace, struct vnode *vp, vm_o
|
||||
object = vp->v_object;
|
||||
error = 0;
|
||||
|
||||
/*
|
||||
* It's necessary to fail if the filsz + offset taken from the
|
||||
* header is greater than the actual file pager object's size.
|
||||
* If we were to allow this, then the vm_map_find() below would
|
||||
* walk right off the end of the file object and into the ether.
|
||||
*
|
||||
* While I'm here, might as well check for something else that
|
||||
* is invalid: filsz cannot be greater than memsz.
|
||||
*/
|
||||
if ((off_t)filsz + offset > object->un_pager.vnp.vnp_size ||
|
||||
filsz > memsz) {
|
||||
uprintf("elf_load_section: truncated ELF file\n");
|
||||
return (ENOEXEC);
|
||||
}
|
||||
|
||||
map_addr = trunc_page((vm_offset_t)vmaddr);
|
||||
file_addr = trunc_page(offset);
|
||||
|
||||
@ -341,6 +357,12 @@ elf_load_file(struct proc *p, const char *file, u_long *addr, u_long *entry)
|
||||
}
|
||||
|
||||
error = exec_map_first_page(imgp);
|
||||
/*
|
||||
* Also make certain that the interpreter stays the same, so set
|
||||
* its VTEXT flag, too.
|
||||
*/
|
||||
if (error == 0)
|
||||
nd.ni_vp->v_flag |= VTEXT;
|
||||
VOP_UNLOCK(nd.ni_vp, 0, p);
|
||||
if (error)
|
||||
goto fail;
|
||||
@ -449,6 +471,15 @@ exec_elf_imgact(struct image_params *imgp)
|
||||
/*
|
||||
* From this point on, we may have resources that need to be freed.
|
||||
*/
|
||||
|
||||
/*
|
||||
* Yeah, I'm paranoid. There is every reason in the world to get
|
||||
* VTEXT now since from here on out, there are places we can have
|
||||
* a context switch. Better safe than sorry; I really don't want
|
||||
* the file to change while it's being loaded.
|
||||
*/
|
||||
atomic_set_long(&imgp->vp->v_flag, VTEXT);
|
||||
|
||||
if ((error = exec_extract_strings(imgp)) != 0)
|
||||
goto fail;
|
||||
|
||||
@ -610,9 +641,6 @@ exec_elf_imgact(struct image_params *imgp)
|
||||
imgp->auxargs = elf_auxargs;
|
||||
imgp->interpreted = 0;
|
||||
|
||||
/* don't allow modifying the file while we run it */
|
||||
imgp->vp->v_flag |= VTEXT;
|
||||
|
||||
fail:
|
||||
return error;
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user