~ chicken-core (chicken-5) 93012bb389d919ee3d575289be1f29d9a5701d8c

commit 93012bb389d919ee3d575289be1f29d9a5701d8c
Author:     Peter Bex <peter@more-magic.net>
AuthorDate: Fri Dec 1 21:29:17 2017 +0100
Commit:     felix <felix@call-with-current-continuation.org>
CommitDate: Sun Dec 3 00:06:59 2017 +0100

    Fix calls to C_reclaim to use the number of saved objects (#1428)
    
    When the argvector change was introduced, several C_reclaim calls were
    incorrectly changed to use the size of the argvector to the C function
    that would invoke C_reclaim.  In many cases we don't save all the
    arguments, which means "c" would be larger than the size of the
    temporary stack.
    
    The value of "c" gets set to C_restart_c, which will be used in the
    setjmp trampoline as the size of the buffer to set aside for the
    re-invoked closure's argvector.  Then, we read C_restart_c words from
    the stack top, which might overshoot the temporary stack bottom, which
    means we're potentially reading beyond malloced memory.
    
    Finally, CHICKEN_initialize would save 2 objects on the temp stack but
    neglected to set C_restart_c to the correct value.  This appeared to
    work only because CHICKEN_run happened to look at the effective stack
    size rather than the restart count.  Now, we use the restart count and
    also assert that it's always identical to the stack size.  Any
    mismatch must be due to a bug.
    
    This is not an exploitable bug because the copied bytes are
    inaccessible: they're lying just beyond the highest argument in the
    argvector, which is inaccessible from Scheme.  The worst that can
    happen is that we try to read from a page that doesn't belong to us
    and we'll segfault.
    
    Signed-off-by: felix <felix@call-with-current-continuation.org>

diff --git a/NEWS b/NEWS
index 10d677b9..f3b00b40 100644
--- a/NEWS
+++ b/NEWS
@@ -167,6 +167,8 @@
   - Correctly calculate memory requirements of Scheme objects produced
     from foreign types with "const" qualifiers, avoiding memory
     corruption (#1424, thanks to Vasilij Schneidermann and Lemonboy)
+  - Do not read beyond temporary stack buffer, which could lead to
+    a crash when returning from a foreign callback (#1428).
 
 4.12.0
 
diff --git a/runtime.c b/runtime.c
index 499fc922..ba79a917 100644
--- a/runtime.c
+++ b/runtime.c
@@ -845,6 +845,7 @@ int CHICKEN_initialize(int heap, int stack, int symbols, void *toplevel)
   C_set_block_item(k0, 0, (C_word)termination_continuation);
   C_save(k0);
   C_save(C_SCHEME_UNDEFINED);
+  C_restart_c = 2;
   return 1;
 }
 
@@ -1536,10 +1537,9 @@ C_word CHICKEN_run(void *toplevel)
     /* We must copy the argvector onto the stack, because
      * any subsequent save() will otherwise clobber it.
      */
-    int argcount = C_temporary_stack_bottom - C_temporary_stack;
-    C_word *p = C_alloc(argcount);
-
-    C_memcpy(p, C_temporary_stack, argcount * sizeof(C_word));
+    C_word *p = C_alloc(C_restart_c);
+    assert(C_restart_c == (C_temporary_stack_bottom - C_temporary_stack));
+    C_memcpy(p, C_temporary_stack, C_restart_c * sizeof(C_word));
     C_temporary_stack = C_temporary_stack_bottom;
     ((C_proc)C_restart_trampoline)(C_restart_c, p);
   }
@@ -2115,6 +2115,7 @@ C_word C_fcall C_callback(C_word closure, int argc)
      * any subsequent save() will otherwise clobber it.
      */
     C_word *p = C_alloc(C_restart_c);
+    assert(C_restart_c == (C_temporary_stack_bottom - C_temporary_stack));
     C_memcpy(p, C_temporary_stack, C_restart_c * sizeof(C_word));
     C_temporary_stack = C_temporary_stack_bottom;
     ((C_proc)C_restart_trampoline)(C_restart_c, p);
@@ -9421,7 +9422,7 @@ void C_ccall C_gc(C_word c, C_word *av)
   }
   else if(f) C_fromspace_top = C_fromspace_limit;
 
-  C_reclaim((void *)gc_2, c);
+  C_reclaim((void *)gc_2, 1);
 }
 
 
@@ -10740,7 +10741,7 @@ void C_ccall C_ensure_heap_reserve(C_word c, C_word *av)
   C_save(k);
 
   if(!C_demand(C_bytestowords(C_unfix(n))))
-    C_reclaim((void *)generic_trampoline, c);
+    C_reclaim((void *)generic_trampoline, 1);
 
   p = C_temporary_stack;
   C_temporary_stack = C_temporary_stack_bottom;
@@ -10764,7 +10765,7 @@ void C_ccall C_return_to_host(C_word c, C_word *av)
 
   return_to_host = 1;
   C_save(k);
-  C_reclaim((void *)generic_trampoline, c);
+  C_reclaim((void *)generic_trampoline, 1);
 }
 
 
@@ -12245,7 +12246,7 @@ void C_ccall C_dump_heap_state(C_word c, C_word *av)
   /* make sure heap is compacted */
   C_save(k);
   C_fromspace_top = C_fromspace_limit; /* force major GC */
-  C_reclaim((void *)dump_heap_state_2, c);
+  C_reclaim((void *)dump_heap_state_2, 1);
 }
 
 
@@ -12473,7 +12474,7 @@ void C_ccall C_filter_heap_objects(C_word c, C_word *av)
   C_save(userarg);
   C_save(func);
   C_fromspace_top = C_fromspace_limit; /* force major GC */
-  C_reclaim((void *)filter_heap_objects_2, c);
+  C_reclaim((void *)filter_heap_objects_2, 4);
 }
 
 C_regparm C_word C_fcall C_i_process_sleep(C_word n)