shyouhei	2019-02-01 16:26:39 +0900 (Fri, 01 Feb 2019)

  New Revision: 66981

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=66981

  Log:
    on-smash canary detection
    
    In addition to detect dead canary, we try to detect the very moment
    when we smash the stack top.  Requested by k0kubun:
    https://twitter.com/k0kubun/status/1085180749899194368

  Modified files:
    trunk/bootstraptest/test_insns.rb
    trunk/vm.c
    trunk/vm_args.c
    trunk/vm_eval.c
    trunk/vm_insnhelper.c
    trunk/vm_insnhelper.h
Index: vm_eval.c
===================================================================
--- vm_eval.c	(revision 66980)
+++ vm_eval.c	(revision 66981)
@@ -116,6 +116,7 @@ vm_call0_body(rb_execution_context_t *ec https://github.com/ruby/ruby/blob/trunk/vm_eval.c#L116
 	    int i;
 
 	    CHECK_VM_STACK_OVERFLOW(reg_cfp, calling->argc + 1);
+            vm_check_canary(ec, reg_cfp->sp);
 
 	    *reg_cfp->sp++ = calling->recv;
 	    for (i = 0; i < calling->argc; i++) {
Index: vm_insnhelper.c
===================================================================
--- vm_insnhelper.c	(revision 66980)
+++ vm_insnhelper.c	(revision 66981)
@@ -201,7 +201,55 @@ vm_check_frame(VALUE type, https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L201
     }
 #undef CHECK
 }
+
+static VALUE vm_stack_canary; /* Initialized later */
+static bool vm_stack_canary_was_born = false;
+
+static void
+vm_check_canary(const rb_execution_context_t *ec, VALUE *sp)
+{
+    const struct rb_control_frame_struct *reg_cfp = ec->cfp;
+    const struct rb_iseq_struct *iseq;
+
+    if (! LIKELY(vm_stack_canary_was_born)) {
+        return; /* :FIXME: isn't it rather fatal to enter this branch?  */
+    }
+    else if (! (iseq = GET_ISEQ())) {
+        return;
+    }
+    else if (LIKELY(sp[0] != vm_stack_canary)) {
+        return;
+    }
+    else {
+        /* we are going to call metods below; squash the canary to
+         * prevent infinite loop. */
+        sp[0] = Qundef;
+    }
+
+    const VALUE *orig = rb_iseq_original_iseq(iseq);
+    const VALUE *encoded = iseq->body->iseq_encoded;
+    const ptrdiff_t pos = GET_PC() - encoded;
+    const enum ruby_vminsn_type insn = (enum ruby_vminsn_type)orig[pos];
+    const char *name = insn_name(insn);
+    const VALUE iseqw = rb_iseqw_new(iseq);
+    const VALUE inspection = rb_inspect(iseqw);
+    const char *stri = rb_str_to_cstr(inspection);
+    const VALUE disasm = rb_iseq_disasm(iseq);
+    const char *strd = "";/* rb_str_to_cstr(disasm); */
+
+    /* rb_bug() is not capable of outputting this large contents.  It
+       is designed to run form a SIGSEGV handler, which tends to be
+       very restricted. */
+    fprintf(stderr,
+        "We are killing the stack canary set by %s, "
+        "at %s@pc=%"PRIdPTR"\n"
+        "watch out the C stack trace.\n"
+        "%s",
+        name, stri, pos, strd);
+    rb_bug("see above.");
+}
 #else
+#define vm_check_canary(ec, sp)
 #define vm_check_frame(a, b, c, d)
 #endif /* VM_CHECK_MODE > 0 */
 
@@ -225,6 +273,7 @@ vm_push_frame(rb_execution_context_t *ec https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L273
 
     /* check stack overflow */
     CHECK_VM_STACK_OVERFLOW0(cfp, sp, local_size + stack_max);
+    vm_check_canary(ec, sp);
 
     ec->cfp = cfp;
 
@@ -2153,6 +2202,7 @@ vm_call_method_missing(rb_execution_cont https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L2202
 
     /* shift arguments: m(a, b, c) #=> method_missing(:m, a, b, c) */
     CHECK_VM_STACK_OVERFLOW(reg_cfp, 1);
+    vm_check_canary(ec, reg_cfp->sp);
     if (argc > 1) {
 	MEMMOVE(argv+1, argv, VALUE, argc-1);
     }
@@ -4087,7 +4137,6 @@ vm_trace(rb_execution_context_t *ec, rb_ https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L4137
 #if VM_CHECK_MODE > 0
 static NORETURN( NOINLINE( COLDFUNC
 void vm_canary_is_found_dead(enum ruby_vminsn_type i, VALUE c)));
-static VALUE vm_stack_canary;
 
 void
 Init_vm_stack_canary(void)
@@ -4095,6 +4144,7 @@ Init_vm_stack_canary(void) https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L4144
     /* This has to be called _after_ our PRNG is properly set up. */
     int n = ruby_fill_random_bytes(&vm_stack_canary, sizeof vm_stack_canary, false);
 
+    vm_stack_canary_was_born = true;
     VM_ASSERT(n == 0);
 }
 
Index: vm_insnhelper.h
===================================================================
--- vm_insnhelper.h	(revision 66980)
+++ vm_insnhelper.h	(revision 66981)
@@ -137,14 +137,22 @@ enum vm_regan_acttype { https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.h#L137
 
 #if VM_CHECK_MODE > 0
 #define SETUP_CANARY() \
-    VALUE * canary; \
+    VALUE *canary; \
     if (leaf) { \
         canary = GET_SP(); \
         SET_SV(vm_stack_canary); \
+    } \
+    else {\
+        SET_SV(Qfalse); /* cleanup */ \
     }
 #define CHECK_CANARY() \
-    if (leaf && (*canary != vm_stack_canary)) { \
-        vm_canary_is_found_dead(INSN_ATTR(bin), *canary); \
+    if (leaf) { \
+        if (*canary == vm_stack_canary) { \
+            *canary = Qfalse; /* cleanup */ \
+        } \
+        else { \
+            vm_canary_is_found_dead(INSN_ATTR(bin), *canary); \
+        } \
     }
 #else
 #define SETUP_CANARY()          /* void */
Index: vm.c
===================================================================
--- vm.c	(revision 66980)
+++ vm.c	(revision 66981)
@@ -1090,6 +1090,7 @@ invoke_iseq_block_from_c(rb_execution_co https://github.com/ruby/ruby/blob/trunk/vm.c#L1090
     stack_check(ec);
 
     CHECK_VM_STACK_OVERFLOW(cfp, argc);
+    vm_check_canary(ec, sp);
     cfp->sp = sp + argc;
     for (i=0; i<argc; i++) {
 	sp[i] = argv[i];
Index: bootstraptest/test_insns.rb
===================================================================
--- bootstraptest/test_insns.rb	(revision 66980)
+++ bootstraptest/test_insns.rb	(revision 66981)
@@ -418,5 +418,5 @@ tests.compact.each {|(insn, expr, *a)| a https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_insns.rb#L418
 # with trace
 tests.compact.each {|(insn, expr, *a)|
   progn = "set_trace_func(proc{})\n" + expr
-  assert_equal 'true', progn, insn, *a
+  assert_equal 'true', progn, 'trace_' + insn, *a
 }
Index: vm_args.c
===================================================================
--- vm_args.c	(revision 66980)
+++ vm_args.c	(revision 66981)
@@ -525,6 +525,7 @@ setup_parameters_complex(rb_execution_co https://github.com/ruby/ruby/blob/trunk/vm_args.c#L525
     VALUE * const orig_sp = ec->cfp->sp;
     unsigned int i;
 
+    vm_check_canary(ec, orig_sp);
     /*
      * Extend SP for GC.
      *
@@ -782,6 +783,7 @@ vm_caller_setup_arg_splat(rb_control_fra https://github.com/ruby/ruby/blob/trunk/vm_args.c#L783
     VALUE *argv = cfp->sp - argc;
     VALUE ary = argv[argc-1];
 
+    vm_check_canary(GET_EC(), cfp->sp);
     cfp->sp--;
 
     if (!NIL_P(ary)) {

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/