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

1 files changed, 31 insertions, 3 deletions

diff --git a/sys/kern/imgact_elf.c b/sys/kern/imgact_elf.c
index cf8267e..52b89e5 100644
--- a/sys/kern/imgact_elf.c
+++ b/sys/kern/imgact_elf.c
@@ -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;
 }