From: ko1@...
Date: 2015-06-17T23:16:37+00:00
Subject: [ruby-core:69640] [Ruby trunk - Bug #10871] Sclass thread unsafe	due to CREF sharing

Issue #10871 has been updated by Koichi Sasada.


Thank you.

I updated a patch.

```diff
Index: insns.def
===================================================================
--- insns.def	(revision 50914)
+++ insns.def	(working copy)
@@ -914,6 +914,8 @@
 (VALUE val)
 {
     VALUE klass;
+    VALUE class_iseq_val = class_iseq->self;
+    rb_iseq_t *orig_class_iseq = NULL;
     rb_vm_defineclass_type_t type = VM_DEFINECLASS_TYPE(flags);
 
     switch (type) {
@@ -963,7 +965,18 @@
       case VM_DEFINECLASS_TYPE_SINGLETON_CLASS:
 	/* val is dummy.  classdef returns class scope value */
 	/* super is dummy */
-	klass = rb_singleton_class(cbase);
+	{
+	    klass = rb_singleton_class(cbase);
+
+	    /* Copy iseq to duplicate cref_stack place.
+	     * This is ad-hoc solution for [Bug #10871].
+	     * and this does not solve more complicated source code with singleton class.
+	     * If you need to solve everything, use Ruby 2.3 and later.
+	     */
+	    orig_class_iseq = class_iseq;
+	    class_iseq_val = rb_iseq_clone(class_iseq->self, cbase);
+	    GetISeqPtr(class_iseq_val, class_iseq);
+	}
 	break;
       case VM_DEFINECLASS_TYPE_MODULE:
 	/* val is dummy.  classdef returns class scope value */
@@ -992,6 +1005,9 @@
     }
 
     COPY_CREF(class_iseq->cref_stack, vm_cref_push(th, klass, NOEX_PUBLIC, NULL));
+    if (orig_class_iseq) {
+	COPY_CREF(orig_class_iseq->cref_stack, vm_cref_push(th, klass, NOEX_PUBLIC, NULL));
+    }
 
     /* enter scope */
     vm_push_frame(th, class_iseq, VM_FRAME_MAGIC_CLASS,
@@ -998,6 +1014,9 @@
 		  klass, 0, VM_ENVVAL_BLOCK_PTR(GET_BLOCK_PTR()),
 		  class_iseq->iseq_encoded, GET_SP(),
 		  class_iseq->local_size, 0, class_iseq->stack_max);
+
+    RB_GC_GUARD(class_iseq_val);
+
     RESTORE_REGS();
     NEXT_INSN();
 }
```


----------------------------------------
Bug #10871: Sclass thread unsafe due to CREF sharing
https://bugs.ruby-lang.org/issues/10871#change-52993

* Author: Evan Phoenix
* Status: Open
* Priority: High
* Assignee: Koichi Sasada
* ruby -v: 2.2.0p0, trunk
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
When entering an sclass, the context is tracked via the same cref mechanism used for class and module, specifically on the iseq->cref_stack. The bug is that the cref_stack is the wrong place to put the new cref because the scope is specific only to that sclass body. Mutating and using the iseq->cref_stack causes any code that reads the cref via this cref_stack to incorrectly pick up the sclass instance instead of the proper scope!

This is major thread safety bug because it means that all uses of `class << obj` are thread-unsafe and can cause random code to fail.

Here is a simple reproduction of the bug: https://gist.github.com/evanphx/6eef92f2c40662a4171b

I attempted to fix the bug by treating an sclass body the same as an eval, which already has special handling for cref's but I don't understand the code enough to make that change quickly.

I believe this is a major bug and hope that ruby-core can address it soon.

Thank you!



-- 
https://bugs.ruby-lang.org/