This is a multipart MIME message.

--==_Exmh_6749825740560
Content-Type: text/plain; charset=us-ascii


Hi -
currently, if more than one thread tries to execute some trampoline
code on the stack, this doesn't work: the process gets a bus error
eventually.
This is doe to the fact that pmap_exec_fixup() maintains a per-
process variable (pm_hiexec) to track changes of the executable
addresses allowed, and that additional permissions are dealt with
lazily, ie only after a fault.
The first thread trying to execute something on the stack gets
dealt with correctly, the second dies.
(This might as well happen with programs using longjmp(); the
inner reason is that the CS is managed in userland.)

Looking at the issue, I found some things which are suboptimal
or which I just don't understand:
-code segment descriptors are used inconsistently: initially
 from the LDT, later from the GDT
-pmap_exec_fixup() will never revoke anything, there is dead code
-pmap_update_pg() is called for exec permission changes. Since
 this is a software-only flag, it looks like a waste.
-the CS in the PCB doesn't seem to ge good for anything

With the appended patches, a single processor works as well as
before for me, and allows multithreaded programs which depend on an
executable stack to run.
Permissions are not revoked for other threads/processors, but
this is not worse than before. (To get this done cleanly, I can
only imagine switching LDTs.)

Comments?

best regards
Matthias


--==_Exmh_6749825740560
Content-Type: text/plain ; name="execstack.txt"; charset=us-ascii
Content-Description: execstack.txt
Content-Disposition: attachment; filename="execstack.txt"

Index: i386/machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/i386/machdep.c,v
retrieving revision 1.545
diff -u -p -r1.545 machdep.c
--- i386/machdep.c	4 Dec 2003 19:38:21 -0000	1.545
+++ i386/machdep.c	5 Dec 2003 18:47:14 -0000
@@ -678,7 +688,7 @@ buildcontext(struct lwp *l, int sel, voi
 	tf->tf_es = GSEL(GUDATA_SEL, SEL_UPL);
 	tf->tf_ds = GSEL(GUDATA_SEL, SEL_UPL);
 	tf->tf_eip = (int)catcher;
-	tf->tf_cs = GSEL(sel, SEL_UPL);
+	tf->tf_cs = LSEL(sel, SEL_UPL);
 	tf->tf_eflags &= ~(PSL_T|PSL_VM|PSL_AC);
 	tf->tf_esp = (int)fp;
 	tf->tf_ss = GSEL(GUDATA_SEL, SEL_UPL);
@@ -691,7 +701,7 @@ sendsig_siginfo(const ksiginfo_t *ksi, c
 	struct proc *p = l->l_proc;
 	struct pmap *pmap = vm_map_pmap(&p->p_vmspace->vm_map);
 	int sel = pmap->pm_hiexec > I386_MAX_EXE_ADDR ?
-	    GUCODEBIG_SEL : GUCODE_SEL;
+	    LUCODEBIG_SEL : LUCODE_SEL;
 	struct sigacts *ps = p->p_sigacts;
 	int onstack;
 	int sig = ksi->ksi_signo;
@@ -789,7 +799,7 @@ cpu_upcall(struct lwp *l, int type, int 
 	tf->tf_es = GSEL(GUDATA_SEL, SEL_UPL);
 	tf->tf_ds = GSEL(GUDATA_SEL, SEL_UPL);
 	tf->tf_cs = pmap->pm_hiexec > I386_MAX_EXE_ADDR ?
-	    GSEL(GUCODEBIG_SEL, SEL_UPL) : GSEL(GUCODE_SEL, SEL_UPL);
+	    LSEL(LUCODEBIG_SEL, SEL_UPL) : LSEL(LUCODE_SEL, SEL_UPL);
 	tf->tf_ss = GSEL(GUDATA_SEL, SEL_UPL);
 	tf->tf_eflags &= ~(PSL_T|PSL_VM|PSL_AC);
 }
Index: i386/pmap.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/i386/pmap.c,v
retrieving revision 1.164
diff -u -p -r1.164 pmap.c
--- i386/pmap.c	3 Nov 2003 04:02:13 -0000	1.164
+++ i386/pmap.c	5 Dec 2003 18:47:14 -0000
@@ -726,9 +726,6 @@ pmap_exec_account(struct pmap *pm, vaddr
 	    pm != vm_map_pmap(&curproc->p_vmspace->vm_map))
 		return;
 
-	if ((opte ^ npte) & PG_X)
-		pmap_update_pg(va);
-
 	/*
 	 * Executability was removed on the last executable change.
 	 * Reset the code segment to something conservative and
@@ -738,9 +735,8 @@ pmap_exec_account(struct pmap *pm, vaddr
 
 	if ((opte & PG_X) && (npte & PG_X) == 0 && va == pm->pm_hiexec) {
 		struct trapframe *tf = curlwp->l_md.md_regs;
-		struct pcb *pcb = &curlwp->l_addr->u_pcb;
 
-		pcb->pcb_cs = tf->tf_cs = GSEL(GUCODE_SEL, SEL_UPL);
+		tf->tf_cs = LSEL(LUCODE_SEL, SEL_UPL);
 		pm->pm_hiexec = I386_MAX_EXE_ADDR;
 	}
 }
@@ -769,15 +765,13 @@ pmap_exec_fixup(struct vm_map *map, stru
 			va = trunc_page(ent->end) - PAGE_SIZE;
 	}
 	vm_map_unlock_read(map);
-	if (va == pm->pm_hiexec)
+	pm->pm_hiexec = va;
+
+	if (tf->tf_cs == LSEL(LUCODEBIG_SEL, SEL_UPL) ||
+	    (pm->pm_hiexec <= I386_MAX_EXE_ADDR))
 		return (0);
 
-	pm->pm_hiexec = va;
-	if (pm->pm_hiexec > I386_MAX_EXE_ADDR) {
-		pcb->pcb_cs = tf->tf_cs = GSEL(GUCODEBIG_SEL, SEL_UPL);
-	} else {
-		pcb->pcb_cs = tf->tf_cs = GSEL(GUCODE_SEL, SEL_UPL);
-	}
+	tf->tf_cs = LSEL(LUCODEBIG_SEL, SEL_UPL);
 	return (1);
 }
 

--==_Exmh_6749825740560--