Merge pull request #2128 from ghaerr/lowstack

[kernel,libc] Rewrite kernel brk/sbrk and stack_check, fix C library malloc

elks/arch/i86/kernel/process.c

@@ -43,34 +43,27 @@ int run_init_process_sptr(const char *cmd, char *sptr, int slen)
 }
 
 /*
- * We only need to do this as long as we support old format binaries
- * that grow stack and heap towards each other
+ * Check that SP is within proper range, called before every syscall.
  */
 void stack_check(void)
 {
-    segoff_t end = current->t_endbrk;
-
-#ifdef CONFIG_EXEC_LOW_STACK
-    if (current->t_begstack <= current->t_enddata) {  /* stack below heap?*/
-        if (current->t_regs.sp < end)
-            return;
-        end = 0;
-    } else
-#endif
-    {
-        /* optional: check stack over min stack*/
-        if (current->t_regs.sp < current->t_begstack - current->t_minstack) {
-          if (current->t_minstack)     /* display if protected stack*/
-            printk("(%P)STACK OUTSIDE PROTECTED LIMIT by %u\n",
-                current->t_begstack - current->t_minstack - current->t_regs.sp);
-        }
-
-        /* check stack overflow heap*/
-        if (current->t_regs.sp > end)
-            return;
+    segoff_t sp = current->t_regs.sp;
+    segoff_t brk = current->t_endbrk;
+    segoff_t stacklow = current->t_begstack - current->t_minstack;
+
+    if (sp < brk) {
+        printk("(%P)STACK OVERFLOW by %u\n", brk - sp);
+        printk("curbreak %u, SP %u\n", current->t_endbrk, current->t_regs.sp);
+        do_exit(SIGSEGV);
+    }
+    if (sp < stacklow) {
+        /* notification only, allow process to continue */
+        printk("(%P)STACK USING %u UNUSED HEAP\n", stacklow - sp);
+    }
+    if (sp > current->t_begstack) {
+        printk("(%P)STACK UNDERFLOW\n");
+        do_exit(SIGSEGV);
     }
-    printk("(%P)STACK OVERFLOW by %u\n", end - current->t_regs.sp);
-    do_exit(SIGSEGV);
 }
 
 /*

elks/arch/i86/mm/malloc.c

@@ -230,49 +230,58 @@ void mm_get_usage (unsigned int * pfree, unsigned int * pused)
 
 // User data segment functions
 
-int sys_brk(segoff_t newbrk)
+static int set_brk(segoff_t brk, int increment)
 {
+    segoff_t newbrk = brk + increment;
+    segoff_t stacklow;
+
     /***unsigned int memfree, memused;
     mm_get_usage(&memfree, &memused);
     printk("brk(%P): new %x, edat %x, ebrk %x, free %x sp %x, eseg %x, %d/%dK\n",
         newbrk, current->t_enddata, current->t_endbrk,
         current->t_regs.sp - current->t_endbrk,
         current->t_regs.sp, current->t_endseg, memfree, memused);***/
 
-    if (newbrk < current->t_enddata)
+    if (newbrk < current->t_enddata) {
+        printk("(%P)SBRK %d FAIL, BELOW HEAP\n", increment);
         return -ENOMEM;
+    }
 
-    if (current->t_begstack > current->t_endbrk) {              /* stack above heap?*/
-        if (newbrk > current->t_begstack - current->t_minstack) {
-            printk("(%d)CAN'T EXPAND HEAP by %u\n",
-                current->pid, newbrk - (current->t_begstack - current->t_minstack));
-            return -ENOMEM;
-        }
+    stacklow = current->t_begstack - current->t_minstack;
+    if (newbrk > stacklow) {
+        printk("(%P)SBRK %d FAIL, OUT OF HEAP SPACE\n", increment);
+        return -ENOMEM;
     }
-#ifdef CONFIG_EXEC_LOW_STACK
-    if (newbrk > current->t_endseg)
+    if (newbrk > current->t_regs.sp) {
+        printk("(%P)SBRK %d FAIL, WOULD OVERWRITE STACK\n", increment);
         return -ENOMEM;
-#endif
-    current->t_endbrk = newbrk;
+    }
 
+    current->t_endbrk = newbrk;
     return 0;
 }
 
+int sys_brk(segoff_t newbrk)
+{
+    dprintk("(%P)BRK %u\n", newbrk);
+    return set_brk(newbrk, 0);
+}
 
-int sys_sbrk (int increment, segoff_t *pbrk)
+int sys_sbrk(int increment, segoff_t *pbrk)
 {
     segoff_t brk = current->t_endbrk;   /* always return start of old break*/
     int err;
 
-    debug("sbrk incr %u pointer %04x curbreak %04x\n", increment, pbrk, brk);
+    if (increment)
+        dprintk("(%P)SBRK %d, curbreak %u, SP %u\n",
+            increment, current->t_endbrk, current->t_regs.sp);
     err = verify_area(VERIFY_WRITE, pbrk, sizeof(*pbrk));
     if (err)
         return err;
-    if (increment) {
-        err = sys_brk(brk + increment);
-        if (err) return err;
-    }
-
+    /* FIXME test for brk+increment overflow/underflow here */
+    err = set_brk(brk, increment);
+    if (err)
+        return err;
     put_user (brk, pbrk);
     return 0;
 }
@@ -307,12 +316,12 @@ int sys_fmemfree(unsigned short segment)
                 seg_free(seg);
                 return 0;
             }
-            debug("sys_fmemfree: not owner %04x\n", segment);
+            printk("sys_fmemfree: not owner %04x\n", segment);
             return -EACCES;
         }
         n = seg->all.next;
     }
-    debug("sys_fmemfree: segment not found %04x\n", segment);
+    printk("sys_fmemfree: segment not found %04x\n", segment);
     return -EINVAL;
 }
 

elks/fs/exec.c

@@ -266,18 +266,8 @@ static int execve_aout(struct inode *inode, struct file *filp, char *sptr, size_
         if (esuph.msh_tbase != 0)
             goto error_exec3;
         base_data = esuph.msh_dbase;
-#ifdef CONFIG_EXEC_LOW_STACK
-        if (base_data & 0xf)
-            goto error_exec3;
         if (base_data != 0)
-            debug("EXEC: New type executable stack = %x\n", base_data);
-
-        if (add_overflow(min_len, base_data, &min_len)) /* adds stack size*/
             goto error_exec3;
-#else
-        if (base_data != 0)
-            goto error_exec3;
-#endif
         break;
 #endif /* CONFIG_EXEC_MMODEL*/
     default:
@@ -293,9 +283,6 @@ static int execve_aout(struct inode *inode, struct file *filp, char *sptr, size_
         goto error_exec3;
     case 1:
         len = min_len;
-#ifdef CONFIG_EXEC_LOW_STACK
-        if (!base_data)
-#endif
         {
             stack = mh.minstack? mh.minstack: INIT_STACK;
             if (add_overflow(len, stack, &len)) {       /* add stack */
@@ -335,14 +322,6 @@ static int execve_aout(struct inode *inode, struct file *filp, char *sptr, size_
         } else {
             stack = INIT_STACK;
             len = min_len;
-#ifdef CONFIG_EXEC_LOW_STACK
-            if (base_data) {
-                if (add_overflow(len, INIT_HEAP, &len)) {
-                    retval = -EFBIG;
-                    goto error_exec3;
-                }
-            } else
-#endif
             {
                 if (add_overflow(len, INIT_HEAP + INIT_STACK, &len)) {
                     retval = -EFBIG;
@@ -495,14 +474,7 @@ static int execve_aout(struct inode *inode, struct file *filp, char *sptr, size_
     currentp->t_enddata = (size_t)mh.dseg + (size_t)mh.bseg + base_data;
     currentp->t_endseg = len;
     currentp->t_regs.dx = currentp->t_minstack = stack;
-
-#ifdef CONFIG_EXEC_LOW_STACK
-    currentp->t_begstack = ((base_data   /* Just above the top of stack */
-        ? base_data
-        : currentp->t_endseg) - slen) & ~1;
-#else
     currentp->t_begstack = (currentp->t_endseg - slen) & ~1; /* force even SP and argv */
-#endif
     fmemcpyb((char *)currentp->t_begstack, seg_data->base, sptr, ds, slen);
 
     finalize_exec(inode, seg_code, seg_data, (word_t)mh.entry, 0);

libc/malloc/Makefile

@@ -4,12 +4,13 @@ COMPILER ?= ia16
 LIB ?= out.a
 
 include $(TOPDIR)/libc/$(COMPILER).inc
-#CFLAGS	+= -DL_alloca
-#CFLAGS	+= -DLAZY_FREE
-# MCHUNK is word not byte min allocation size
-CFLAGS	+= -DMCHUNK=512
+# allocations smaller than MCHUNK words (not bytes) are rounded up,
+# larger requests are allocated from heap as is.
+CFLAGS	+= -DMCHUNK=16
 #CFLAGS	+= -DMINALLOC
+#CFLAGS	+= -DLAZY_FREE
 #CFLAGS	+= -DVERBOSE
+#CFLAGS	+= -DL_alloca
 
 # use V7 malloc for heap integrity checking
 #OBJS = v7malloc.o calloc.o sbrk.o brk.o

libc/malloc/__mini_malloc.c

@@ -8,12 +8,16 @@ void __wcnear *
 __mini_malloc(size_t size)
 {
 	mem __wcnear *ptr;
-	size_t sz;
 
+#if 0
+	size_t sz;
 	/* First time round this _might_ be odd, But we won't do that! */
 	sz = (size_t)sbrk(0);
 	if(sz & (sizeof(mem) - 1))
 		sbrk(4 - (sz & (sizeof(mem) - 1)));
+#endif
+
+	size += sizeof(mem) * 2 - 1;	/* Round up and leave space for size field */
 
 	/* Minor oops here, sbrk has a signed argument */
 	if((int)size <= 0 || size > (((unsigned)-1) >> 1) - sizeof(mem) * 3)
@@ -22,9 +26,7 @@ __mini_malloc(size_t size)
 		return 0;
 	}
 
-	size += sizeof(mem) * 2 - 1;	/* Round up and leave space for size field */
 	size /= sizeof(mem);
-
 	ptr = (mem __wcnear *) sbrk(size * sizeof(mem));
 	/*if((uintptr_t)ptr == (intptr_t)-1)*/  /* this is better only when not __wcnear */
     if ((int)ptr == -1)

libc/malloc/malloc.c

@@ -265,10 +265,13 @@ malloc(size_t size)
       {
 #ifdef MCHUNK
          unsigned int alloc;
-         alloc = sizeof(mem) * (MCHUNK * ((sz + MCHUNK - 1) / MCHUNK) - 1);
+         if (sz < MCHUNK)
+                alloc = sizeof(mem) * (MCHUNK * ((sz + MCHUNK - 1) / MCHUNK) - 1);
+         else alloc = sz * sizeof(mem);
 	 ptr = __mini_malloc(alloc);
 	 if (ptr)
 	    __insert_chunk(ptr - 1);
+#if MCHUNK >= 256
 	 else		/* Oooo, near end of RAM */
 	 {
 	    unsigned int needed = alloc;
@@ -283,6 +286,7 @@ malloc(size_t size)
 	       else     alloc/=2;
 	    }
 	 }
+#endif
 	 ptr = __search_chunk(sz);
 	 if (ptr == 0)
 #endif