bpo-46417: Fix race condition on setting type __bases__ (GH-30788)

Fix a race condition on setting a type __bases__ attribute: the
internal function add_subclass() now gets the
PyTypeObject.tp_subclasses member after calling PyWeakref_NewRef()
which can trigger a garbage collection which can indirectly modify
PyTypeObject.tp_subclasses.

diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-01-22-14-39-23.bpo-46417.3U5SfN.rst b/Misc/NEWS.d/next/Core and Builtins/2022-01-22-14-39-23.bpo-46417.3U5SfN.rst
new file mode 100644 (file)
index 0000000..54fe09b
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2022-01-22-14-39-23.bpo-46417.3U5SfN.rst
@@ -0,0 +1,5 @@
+Fix a race condition on setting a type ``__bases__`` attribute: the internal
+function ``add_subclass()`` now gets the ``PyTypeObject.tp_subclasses``
+member after calling :c:func:`PyWeakref_NewRef` which can trigger a garbage
+collection which can indirectly modify ``PyTypeObject.tp_subclasses``. Patch
+by Victor Stinner.

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index e4a4824fa2e41bbd1e538ac9439aca8b73426f31..2b47afe30e6ecbd2fd98e4a5368d29eee9db15dc 100644 (file)
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -6503,24 +6503,29 @@ PyType_Ready(PyTypeObject *type)
 static int
 add_subclass(PyTypeObject *base, PyTypeObject *type)
 {
-    int result = -1;
-    PyObject *dict, *key, *newobj;
+    PyObject *key = PyLong_FromVoidPtr((void *) type);
+    if (key == NULL)
+        return -1;
 
-    dict = base->tp_subclasses;
+    PyObject *ref = PyWeakref_NewRef((PyObject *)type, NULL);
+    if (ref == NULL) {
+        Py_DECREF(key);
+        return -1;
+    }
+
+    // Only get tp_subclasses after creating the key and value.
+    // PyWeakref_NewRef() can trigger a garbage collection which can execute
+    // arbitrary Python code and so modify base->tp_subclasses.
+    PyObject *dict = base->tp_subclasses;
     if (dict == NULL) {
         base->tp_subclasses = dict = PyDict_New();
         if (dict == NULL)
             return -1;
     }
     assert(PyDict_CheckExact(dict));
-    key = PyLong_FromVoidPtr((void *) type);
-    if (key == NULL)
-        return -1;
-    newobj = PyWeakref_NewRef((PyObject *)type, NULL);
-    if (newobj != NULL) {
-        result = PyDict_SetItem(dict, key, newobj);
-        Py_DECREF(newobj);
-    }
+
+    int result = PyDict_SetItem(dict, key, ref);
+    Py_DECREF(ref);
     Py_DECREF(key);
     return result;
 }