Date: Wed, 29 May 2002 16:04:34 -0400 From: Jake Burkholder <jake@locore.ca> To: Archie Cobbs <archie@dellroad.org> Cc: freebsd-arch@FreeBSD.ORG Subject: Re: Kernel stack overflow detection? Message-ID: <20020529160434.P62759@locore.ca> In-Reply-To: <200205291920.g4TJKkE92786@arch20m.dellroad.org>; from archie@dellroad.org on Wed, May 29, 2002 at 12:20:45PM -0700 References: <20020528221812.O62759@locore.ca> <200205291920.g4TJKkE92786@arch20m.dellroad.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Apparently, On Wed, May 29, 2002 at 12:20:45PM -0700, Archie Cobbs said words to the effect of; > 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). -current just leaves the page unmapped which has the same effect and doesn't waste a physical page for it. This is slightly eaiser to do if the stack is moved in the u. area. I think sparc64 is the only architecture that uses a guard page for proc0's kstack; this is worth doing in -current for i386 at least. > > 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?20020529160434.P62759>