--TB36FDmn/VVEgNH/
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

okay...here's what (i think) is happening.

since the permissions of the stack changed (from rwx to rw- when we
got the w^x code), there's a greater possibility of things getting
merged with the stack (the merge code is eager to merge stuff that can
be merged), but with the way that stack faults (ie, attempts to write
to unmapped pages) are handled, attempting to write to something at
the "top" (ie, the lowest address) of the stack, not incurring page
faults in an orderly manner makes you die.

enami-san, your change makes sense in a general case, which handles
running up against the header entry in the circular list much more
sensible, but there's still the "merge with stack" problem.

i've attached a patch that implements a "stack" flag for map entries.
entries that are for stack space are not mergeable with non-stack
entries, and vice versa.  either of our patches solve this specific
problem (indeed, as i said, yours makes the code somewhat cleaner,
too), but in the general case, i think they should both be applied.

fwiw, i initially wrote the stack code about six months ago after
arguing with an openbsd hacker over many beers, but was never
convinced that it actually solved a real problem, only a theoretical
one.  :)

lemme know what you think.

thanks.

(hmm...i still need to patch the linux_exec_setup_stack() function)

-- 
|-----< "CODE WARRIOR" >-----|
codewarrior@daemon.org             * "ah!  i see you have the internet
twofsonet@graffiti.com (Andrew Brown)                that goes *ping*!"
werdna@squooshy.com       * "information is power -- share the wealth."

--TB36FDmn/VVEgNH/
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="uvm.stack.patch"

Index: kern/exec_subr.c
===================================================================
RCS file: /cvsroot/src/sys/kern/exec_subr.c,v
retrieving revision 1.41
diff -u -r1.41 exec_subr.c
--- kern/exec_subr.c	2003/08/29 01:44:02	1.41
+++ kern/exec_subr.c	2003/09/23 00:08:55
@@ -254,6 +254,7 @@
 {
 	int error;
 	long diff;
+	const int stack = cmd->ev_flags & VMCMD_STACK ? UVM_FLAG_STACK : 0;
 
 	diff = cmd->ev_addr - trunc_page(cmd->ev_addr);
 	cmd->ev_addr -= diff;			/* required by uvm_map */
@@ -263,7 +264,7 @@
 			round_page(cmd->ev_len), NULL, UVM_UNKNOWN_OFFSET, 0,
 			UVM_MAPFLAG(cmd->ev_prot, UVM_PROT_ALL, UVM_INH_COPY,
 			UVM_ADV_NORMAL,
-			UVM_FLAG_FIXED|UVM_FLAG_COPYONW));
+			UVM_FLAG_FIXED|UVM_FLAG_COPYONW|stack));
 	return error;
 }
 
@@ -339,12 +340,13 @@
 	noaccess_linear_min = (u_long)STACK_ALLOC(STACK_GROW(epp->ep_minsaddr, 
 	    access_size), noaccess_size);
 	if (noaccess_size > 0) {
-		NEW_VMCMD(&epp->ep_vmcmds, vmcmd_map_zero, noaccess_size,
-		    noaccess_linear_min, NULL, 0, VM_PROT_NONE);
+		NEW_VMCMD2(&epp->ep_vmcmds, vmcmd_map_zero, noaccess_size,
+		    noaccess_linear_min, NULL, 0, VM_PROT_NONE, VMCMD_STACK);
 	}
 	KASSERT(access_size > 0);
-	NEW_VMCMD(&epp->ep_vmcmds, vmcmd_map_zero, access_size,
-	    access_linear_min, NULL, 0, VM_PROT_READ | VM_PROT_WRITE);
+	NEW_VMCMD2(&epp->ep_vmcmds, vmcmd_map_zero, access_size,
+	    access_linear_min, NULL, 0, VM_PROT_READ | VM_PROT_WRITE,
+	    VMCMD_STACK);
 
 	return 0;
 }
Index: sys/exec.h
===================================================================
RCS file: /cvsroot/src/sys/sys/exec.h,v
retrieving revision 1.101
diff -u -r1.101 exec.h
--- sys/exec.h	2003/08/29 01:44:03	1.101
+++ sys/exec.h	2003/09/23 00:09:02
@@ -200,6 +200,7 @@
 #define	VMCMD_RELATIVE	0x0001	/* ev_addr is relative to base entry */
 #define	VMCMD_BASE	0x0002	/* marks a base entry */
 #define	VMCMD_FIXED	0x0004	/* entry must be mapped at ev_addr */
+#define	VMCMD_STACK	0x0008	/* this is a stack segment */
 };
 
 #ifdef _KERNEL
Index: uvm/uvm.h
===================================================================
RCS file: /cvsroot/src/sys/uvm/uvm.h,v
retrieving revision 1.35
diff -u -r1.35 uvm.h
--- uvm/uvm.h	2002/12/01 22:58:43	1.35
+++ uvm/uvm.h	2003/09/23 00:09:03
@@ -134,11 +134,13 @@
 #define UVM_ET_SUBMAP		0x02	/* it is a vm_map submap */
 #define UVM_ET_COPYONWRITE 	0x04	/* copy_on_write */
 #define UVM_ET_NEEDSCOPY	0x08	/* needs_copy */
+#define UVM_ET_STACK		0x10	/* it is a stack segment */
 
 #define UVM_ET_ISOBJ(E)		(((E)->etype & UVM_ET_OBJ) != 0)
 #define UVM_ET_ISSUBMAP(E)	(((E)->etype & UVM_ET_SUBMAP) != 0)
 #define UVM_ET_ISCOPYONWRITE(E)	(((E)->etype & UVM_ET_COPYONWRITE) != 0)
 #define UVM_ET_ISNEEDSCOPY(E)	(((E)->etype & UVM_ET_NEEDSCOPY) != 0)
+#define UVM_ET_ISSTACK(E)	(((E)->etype & UVM_ET_STACK) != 0)
 
 #ifdef _KERNEL
 
Index: uvm/uvm_extern.h
===================================================================
RCS file: /cvsroot/src/sys/uvm/uvm_extern.h,v
retrieving revision 1.84
diff -u -r1.84 uvm_extern.h
--- uvm/uvm_extern.h	2003/08/11 16:33:30	1.84
+++ uvm/uvm_extern.h	2003/09/23 00:09:04
@@ -143,6 +143,7 @@
 #define UVM_FLAG_AMAPPAD 0x100000 /* for bss: pad amap to reduce malloc() */
 #define UVM_FLAG_TRYLOCK 0x200000 /* fail if we can not lock map */
 #define UVM_FLAG_NOWAIT  0x400000 /* not allowed to sleep */
+#define UVM_FLAG_STACK   0x800000 /* this is stack */
 
 /* macros to extract info */
 #define UVM_PROTECTION(X)	((X) & UVM_PROT_MASK)
Index: uvm/uvm_map.c
===================================================================
RCS file: /cvsroot/src/sys/uvm/uvm_map.c,v
retrieving revision 1.137
diff -u -r1.137 uvm_map.c
--- uvm/uvm_map.c	2003/08/26 15:12:18	1.137
+++ uvm/uvm_map.c	2003/09/23 00:09:11
@@ -651,9 +651,8 @@
 	}
 
 	/*
-	 * try and insert in map by extending previous entry, if possible.
-	 * XXX: we don't try and pull back the next entry.   might be useful
-	 * for a stack, but we are currently allocating our stack in advance.
+	 * try to insert into map by extending previous entry or next entry,
+	 * if possible.  or both, if we're really lucky.
 	 */
 
 	if (flags & UVM_FLAG_NOMERGE)
@@ -678,6 +677,10 @@
 		    prev_entry->advice != advice)
 			goto forwardmerge;
 
+		if (((flags & UVM_FLAG_STACK) != 0) !=
+		    UVM_ET_ISSTACK(prev_entry))
+			goto forwardmerge;
+
 		/* wiring status must match (new area is unwired) */
 		if (VM_MAPENT_ISWIRED(prev_entry))
 			goto forwardmerge;
@@ -749,6 +752,10 @@
 		    prev_entry->next->advice != advice)
 			goto nomerge;
 
+		if (((flags & UVM_FLAG_STACK) != 0) !=
+		    UVM_ET_ISSTACK(prev_entry))
+			goto nomerge;
+
 		/* wiring status must match (new area is unwired) */
 		if (VM_MAPENT_ISWIRED(prev_entry->next))
 			goto nomerge;
@@ -907,6 +914,8 @@
 
 		if (uobj)
 			new_entry->etype = UVM_ET_OBJ;
+		else if (flags & UVM_FLAG_STACK)
+			new_entry->etype = UVM_ET_STACK;
 		else
 			new_entry->etype = 0;
 
@@ -1208,8 +1217,7 @@
 			next = entry->next;
 		} else
 			next = entry->prev;
-		if (next == &map->header ||
-		    (!topdown && next->start >= end) ||
+		if ((!topdown && next->start >= end) ||
 		    ( topdown && next->end   <= hint))
 			break;
 		if (flags & UVM_FLAG_FIXED) {

--TB36FDmn/VVEgNH/--