[Python-checkins] bpo-46417: Fix race condition on setting type __bases__ (GH-30788) (GH-30789)

[vstinner]

https://github.com/python/cpython/commit/acda9f3b90c33e4020237cb9e5c676efb38f7847
commit: acda9f3b90c33e4020237cb9e5c676efb38f7847
branch: 3.10
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: vstinner <vstinner at python.org>
date: 2022-01-22T15:28:36+01:00
summary:

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

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.
(cherry picked from commit f1c6ae3270913e095d24ae13ecf96f5a32c8c503)

Co-authored-by: Victor Stinner <vstinner at python.org>

Co-authored-by: Victor Stinner <vstinner at python.org>

files:
A Misc/NEWS.d/next/Core and Builtins/2022-01-22-14-39-23.bpo-46417.3U5SfN.rst
M Objects/typeobject.c

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
index 0000000000000..54fe09b7ba454
--- /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 b23e36a420fa0..10b69fe1196db 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -6384,24 +6384,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;
 }