Re: latest openjdk7 triggers kernel panic

On Thu, Dec 26, 2013 at 02:04:17PM +0200, Andriy Gapon wrote:
> 
> I am running FreeBSD based on the head from a few weeks ago, amd64.
> 
> It seems that after a recent upgrade of openjdk7 I consistently get a kernel
> panic when a java process starts:
> 
> panic: Bad entry start/end for new stack entry
> KDB: stack backtrace:
> db_trace_self_wrapper() at 0xffffffff803adc9b = db_trace_self_wrapper+0x2b/frame
> 0xfffffe02ba6fe6e0
> kdb_backtrace() at 0xffffffff805cbd79 = kdb_backtrace+0x39/frame 0xfffffe02ba6fe790
> panic() at 0xffffffff80597733 = panic+0x1a3/frame 0xfffffe02ba6fe810
> vm_map_stack() at 0xffffffff80719f2e = vm_map_stack+0x3ce/frame 0xfffffe02ba6fe8a0
> vm_mmap() at 0xffffffff8071c270 = vm_mmap+0x520/frame 0xfffffe02ba6fea30
> sys_mmap() at 0xffffffff8071bad3 = sys_mmap+0x303/frame 0xfffffe02ba6feaf0
> amd64_syscall() at 0xffffffff8074d0c8 = amd64_syscall+0x238/frame 0xfffffe02ba6febf0
> Xfast_syscall() at 0xffffffff80733e2b = Xfast_syscall+0xfb/frame 0xfffffe02ba6febf0
> 
> Specifically, new_entry->end != top condition is true.
> new_entry->end is consistently greater than top by 3 pages.
> 
> I suspect that java now does some hacky things with its stack and I suspect that
> vm_map_simplify_entry() call at the end of vm_map_insert() could be to blame.
> Although, the call is guarded by a check:
> 
> 1290         /*
> 1291          * It may be possible to merge the new entry with the next and/or
> 1292          * previous entries.  However, due to MAP_STACK_* being a hack, a
> 1293          * panic can result from merging such entries.
> 1294          */
> 1295         if ((cow & (MAP_STACK_GROWS_DOWN | MAP_STACK_GROWS_UP)) == 0)
> 1296                 vm_map_simplify_entry(map, new_entry);
> 
> But that check seems to be defeated by the fact that vm_map_stack() clears our
> the relevant bits after saving them locally:
> 
> 3335         /*
> 3336          * The stack orientation is piggybacked with the cow argument.
> 3337          * Extract it into orient and mask the cow argument so that we
> 3338          * don't pass it around further.
> 3339          * NOTE: We explicitly allow bi-directional stacks.
> 3340          */
> 3341         orient = cow & (MAP_STACK_GROWS_DOWN|MAP_STACK_GROWS_UP);
> 3342         cow &= ~orient;
> 

I provided the following patch yesterday to Peter, after the similar mail.

diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c
index e3842a3..51b60de 100644
--- a/sys/vm/vm_map.c
+++ b/sys/vm/vm_map.c
_at__at_ -1207,6 +1207,7 _at__at_ charged:
 	}
 	else if ((prev_entry != &map->header) &&
 		 (prev_entry->eflags == protoeflags) &&
+		 (cow & (MAP_ENTRY_GROWS_DOWN | MAP_ENTRY_GROWS_UP)) == 0 &&
 		 (prev_entry->end == start) &&
 		 (prev_entry->wired_count == 0) &&
 		 (prev_entry->cred == cred ||

But there were no core dump available, and I think that your note is also
valid.  I do not see why cow should be cleared from the stack direction
bits.

diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c
index e3842a3..e15e0ca 100644
--- a/sys/vm/vm_map.c
+++ b/sys/vm/vm_map.c
_at__at_ -3339,7 +3340,6 _at__at_ vm_map_stack(vm_map_t map, vm_offset_t addrbos, vm_size_t max_ssize,
 	 * NOTE: We explicitly allow bi-directional stacks.
 	 */
 	orient = cow & (MAP_STACK_GROWS_DOWN|MAP_STACK_GROWS_UP);
-	cow &= ~orient;
 	KASSERT(orient != 0, ("No stack grow direction"));
 
 	if (addrbos < vm_map_min(map) ||