Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 29 May 2002 12:20:45 -0700 (PDT)
From:      Archie Cobbs <archie@dellroad.org>
To:        Jake Burkholder <jake@locore.ca>
Cc:        freebsd-arch@FreeBSD.ORG
Subject:   Re: Kernel stack overflow detection?
Message-ID:  <200205291920.g4TJKkE92786@arch20m.dellroad.org>
In-Reply-To: <20020528221812.O62759@locore.ca> "from Jake Burkholder at May 28, 2002 10:18:12 pm"

next in thread | previous in thread | raw e-mail | index | archive | help
Jake Burkholder writes:
> > If INVARIANTS doesn't do so already, I'd like to propose to write
> > up an INVARIANTS check that would validate that the kernel stack
> > has not overflowed. However I'm curious if anyone has done this
> > already and/or what the right way to go about it would be. E.g, add
> > an extra stack page with read-only protection? Any hints appreciated.
> 
> -current has a guard page, -stable does not.   Also, in -current the
> u. area and the pcb were moved so the kernel stack grows away from
> them, instead of towards.  Either of these changes should be relatively
> easy to back port.
> 
> Note that on x86 a page fault due a stack overflow will cause a double
> fault; the double fault handler uses a task gate which does a hardware
> context switch to get off of the bad stack.

So here's a first attempt at a patch for -stable. This patch does
not try to be too ambitious:

  - It doesn't move the relative position of the stack in the u. area
  - It only works for i386

This is a MFC of the -current option KSTACK_GUARD although it's
done differently (-current and -stable are different anyway in
this area).

It appears that the -current KSTACK_GUARD doesn't mark the page
as read-only (?), whereas this patch does. The advantage is automated
hardware detection of stack overflow, but the disadvantage is
that we have to increase UPAGES by 2 instead of 1 to avoid making
the writable portion of the stack smaller (because before it was
not a multiple of PAGE_SIZE).

Eventually, this option can be applied to other architectures and
moved from conf/options.i386 into conf/options.

Thanks for any reviews/comments.

-Archie

__________________________________________________________________________
Archie Cobbs     *     Packet Design     *     http://www.packetdesign.com

--- i386/i386/locore.s	Thu Sep 20 02:29:23 2001
+++ i386/i386/locore.s	Wed May 29 11:30:20 2002
@@ -813,9 +813,34 @@
 	fillkptphys($PG_RW)
 
 /* Map proc0's UPAGES in the physical way ... */
+#ifdef KSTACK_GUARD
+	/*
+	 * Map the 1st and the 3rd UPAGES pages as writable and the 2nd
+	 * page as read-only to detect kernel stack overflows.
+	 *
+	 * Because of the way fillkptphys() works we have to do this in
+	 * three stages: 1st page RW, 2nd page RO, and pages 3-N RW.
+	 */
+	movl	R(p0upa), %eax
+	movl	$1, %ecx
+	fillkptphys($PG_RW)
+
+	movl	R(p0upa), %eax
+	addl	$PAGE_SIZE, %eax
+	movl	$1, %ecx
+	fillkptphys($PG_V)
+
+	movl	R(p0upa), %eax
+	addl	$PAGE_SIZE, %eax
+	addl	$PAGE_SIZE, %eax
+	movl	$UPAGES, %ecx
+	subl	$2, %ecx
+	fillkptphys($PG_RW)
+#else
 	movl	R(p0upa), %eax
 	movl	$UPAGES, %ecx
 	fillkptphys($PG_RW)
+#endif
 
 /* Map ISA hole */
 	movl	$ISA_HOLE_START, %eax
--- i386/i386/pmap.c	Thu Jan  3 17:17:33 2002
+++ i386/i386/pmap.c	Wed May 29 11:45:39 2002
@@ -890,7 +890,17 @@
 		/*
 		 * Enter the page into the kernel address space.
 		 */
+#ifdef KSTACK_GUARD
+		/* Make bottom page of stack area un-writable */
+		if (i == 1)
+			*(ptek + i) = VM_PAGE_TO_PHYS(m) | PG_V | pgeflag;
+		else {
+			*(ptek + i) = VM_PAGE_TO_PHYS(m)
+			    | PG_RW | PG_V | pgeflag;
+		}
+#else
 		*(ptek + i) = VM_PAGE_TO_PHYS(m) | PG_RW | PG_V | pgeflag;
+#endif
 		if (oldpte) {
 			if ((oldpte & PG_G) || (cpu_class > CPUCLASS_386)) {
 				invlpg((vm_offset_t) up + i * PAGE_SIZE);
@@ -901,7 +911,15 @@
 
 		vm_page_wakeup(m);
 		vm_page_flag_clear(m, PG_ZERO);
+#ifdef KSTACK_GUARD
+		/* Make bottom page of stack area un-writable */
+		if (i == 1)
+			vm_page_flag_set(m, PG_MAPPED);
+		else
+			vm_page_flag_set(m, PG_MAPPED | PG_WRITEABLE);
+#else
 		vm_page_flag_set(m, PG_MAPPED | PG_WRITEABLE);
+#endif
 		m->valid = VM_PAGE_BITS_ALL;
 	}
 	if (updateneeded)
@@ -997,7 +1015,15 @@
 
 		vm_page_wire(m);
 		vm_page_wakeup(m);
+#ifdef KSTACK_GUARD
+		/* Make bottom page of stack area un-writable */
+		if (i == 1)
+			vm_page_flag_set(m, PG_MAPPED);
+		else
+			vm_page_flag_set(m, PG_MAPPED | PG_WRITEABLE);
+#else
 		vm_page_flag_set(m, PG_MAPPED | PG_WRITEABLE);
+#endif
 	}
 }
 
--- i386/include/param.h	Mon Sep 24 23:14:07 2001
+++ i386/include/param.h	Wed May 29 11:34:56 2002
@@ -110,7 +110,11 @@
 #define MAXDUMPPGS	(DFLTPHYS/PAGE_SIZE)
 
 #define IOPAGES	2		/* pages of i/o permission bitmap */
+#ifdef KSTACK_GUARD
+#define UPAGES	5		/* pages of u-area + kernel stack guard */
+#else
 #define UPAGES	3		/* pages of u-area */
+#endif
 
 /*
  * Ceiling on amount of swblock kva space.
--- conf/options.i386	Mon Dec 10 04:17:04 2001
+++ conf/options.i386	Wed May 29 11:32:38 2002
@@ -30,6 +30,9 @@
 COMPAT_SVR4		opt_dontuse.h
 DEBUG_SVR4		opt_svr4.h
 
+# Kernel stack guard
+KSTACK_GUARD		opt_global.h
+
 # i386 SMP options
 APIC_IO			opt_global.h
 

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200205291920.g4TJKkE92786>