Stackless issue python#98: Prevent an unlikely NULL-pointer access in safe_pickle.c pickle_callback().

Anselm Kruis · Anselm Kruis · commit f2607c93f694 · 2016-11-06T23:50:47.000+01:00
The bug is very old and might affect multiprocessing. The change fixes the bug and adds a test case. https://bitbucket.org/stackless-dev/stackless/issues/98 (grafted from 7cd2a4e39aa757427ccbf176ef7a0cd6f8f1514e)
diff --git a/Stackless/changelog.txt b/Stackless/changelog.txt
@@ -9,6 +9,10 @@ What's New in Stackless 3.X.X?
 
 *Release date: 20XX-XX-XX*
 
+- https://bitbucket.org/stackless-dev/stackless/issues/98
+  Prevent an unlikely NULL-pointer access in safe_pickle.c pickle_callback().
+  The bug is very old and might affect multiprocessing.
+
 - https://bitbucket.org/stackless-dev/stackless/issues/96
   Impose a very high limit on the recursion depth of cPickle.
   Previously an infinite recursion could eat up all you memory
diff --git a/Stackless/pickling/safe_pickle.c b/Stackless/pickling/safe_pickle.c
@@ -31,8 +31,12 @@ pickle_callback(PyFrameObject *f, int exc, PyObject *retval)
      */
     saved_base = ts->st.cstack_root;
     ts->st.cstack_root = STACK_REFPLUS + (intptr_t *) &f;
-    Py_DECREF(retval);
-    cf->i = cPickle_save(cf->ob1, cf->ob2, cf->n);
+    if (retval) {
+        Py_DECREF(retval);
+        cf->i = cPickle_save(cf->ob1, cf->ob2, cf->n);
+    } else {
+        cf->i = -1;
+    }
     ts->st.cstack_root = saved_base;
 
     /* jump back. No decref, frame contains result. */
@@ -41,6 +45,7 @@ pickle_callback(PyFrameObject *f, int exc, PyObject *retval)
     ts->frame = cf->f_back;
     slp_transfer_return(cst);
     /* never come here */
+    assert(0);
     return NULL;
 }
 
diff --git a/Stackless/unittests/test_defects.py b/Stackless/unittests/test_defects.py
@@ -4,6 +4,8 @@
 import gc
 import sys
 import types
+from io import BytesIO
+import time
 try:
     import threading
     withThreads = True
@@ -428,6 +430,64 @@ def test_invalid_args_tasklet_kill(self):
         self.assertRaises(TypeError, func, False, None)
 
 
+class TestCPickleBombHandling_Dict(dict):
+    pass
+
+
+class TestCPickleBombHandling_Cls(object):
+    def __getstate__(self):
+        try:
+            started = self.started
+        except AttributeError:
+            pass
+        else:
+            self.started = None
+            started.set()
+        # print("started")
+        time.sleep(0.05)  # give the other thread a chance to run
+        return self.__dict__
+
+
+class TestCPickleBombHandling(StacklessTestCase):
+    def other_thread(self, pickler, c):
+        try:
+            pickler.dump(c)
+        except TaskletExit:
+            self.killed = None
+        except:
+            self.killed = sys.exc_info()
+        else:
+            self.killed = False
+
+    def test_kill_during_cPickle_stack_switch(self):
+        # this test kills the main/current tasklet of a other-thread,
+        # which is fast-pickling a recursive structure. This leads to an
+        # infinite recursion, which gets interrupted by a bomb thrown from
+        # main-thread. Until issue #98 got fixed, this caused a crash.
+        # See https://bitbucket.org/stackless-dev/stackless/issues/98
+        buf = BytesIO()
+        import _pickle as pickle
+        pickler = pickle.Pickler(buf, protocol=-1)
+        pickler.fast = 1
+
+        started = threading.Event()
+
+        c = TestCPickleBombHandling_Cls()
+        c.started = started
+        d = TestCPickleBombHandling_Dict()
+        d[1] = d
+        c.recursive = d
+        self.killed = "undefined"
+        t = threading.Thread(target=self.other_thread, name="other_thread", args=(pickler, c))
+        t.start()
+        started.wait()
+        stackless.get_thread_info(t.ident)[0].kill(pending=True)
+        # print("killing")
+        t.join()
+        if isinstance(self.killed, tuple):
+            raise (self.killed[0], self.killed[1], self.killed[2])
+        self.assertIsNone(self.killed)
+
 if __name__ == '__main__':
     if not sys.argv[1:]:
         sys.argv.append('-v')
