]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-130555: Fix use-after-free in dict.clear() with embedded values (gh-145268)
authorSam Gross <colesbury@gmail.com>
Mon, 2 Mar 2026 17:25:13 +0000 (12:25 -0500)
committerGitHub <noreply@github.com>
Mon, 2 Mar 2026 17:25:13 +0000 (12:25 -0500)
Lib/test/test_dict.py
Misc/NEWS.d/next/Core_and_Builtins/2026-02-26-12-00-00.gh-issue-130555.TMSOIu.rst [new file with mode: 0644]
Objects/dictobject.c

index 8a4e19c95a36efb66599c14b93dd019b8359d47f..f28155fe50148d2cee340670a47131ae48fdadb8 100644 (file)
@@ -1680,6 +1680,69 @@ class DictTest(unittest.TestCase):
         self.assertEqual(len(d), len(items), d)
         self.assertEqual(d, dict(items))
 
+    def test_clear_reentrant_embedded(self):
+        # gh-130555: dict.clear() must be safe when values are embedded
+        # in an object and a destructor mutates the dict.
+        class MyObj: pass
+        class ClearOnDelete:
+            def __del__(self):
+                nonlocal x
+                del x
+
+        x = MyObj()
+        x.a = ClearOnDelete()
+
+        d = x.__dict__
+        d.clear()
+
+    def test_clear_reentrant_cycle(self):
+        # gh-130555: dict.clear() must be safe for embedded dicts when the
+        # object is part of a reference cycle and the last reference to the
+        # dict is via the cycle.
+        class MyObj: pass
+        obj = MyObj()
+        obj.f = obj
+        obj.attr = "attr"
+
+        d = obj.__dict__
+        del obj
+
+        d.clear()
+
+    def test_clear_reentrant_force_combined(self):
+        # gh-130555: dict.clear() must be safe when a destructor forces the
+        # dict from embedded/split to combined (setting ma_values to NULL).
+        class MyObj: pass
+        class ForceConvert:
+            def __del__(self):
+                d[1] = "trigger"
+
+        x = MyObj()
+        x.a = ForceConvert()
+        x.b = "other"
+
+        d = x.__dict__
+        d.clear()
+
+    def test_clear_reentrant_delete(self):
+        # gh-130555: dict.clear() must be safe when a destructor deletes
+        # a key from the same embedded dict.
+        class MyObj: pass
+        class DelKey:
+            def __del__(self):
+                try:
+                    del d['b']
+                except KeyError:
+                    pass
+
+        x = MyObj()
+        x.a = DelKey()
+        x.b = "value_b"
+        x.c = "value_c"
+
+        d = x.__dict__
+        d.clear()
+
 
 class CAPITest(unittest.TestCase):
 
diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-02-26-12-00-00.gh-issue-130555.TMSOIu.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-02-26-12-00-00.gh-issue-130555.TMSOIu.rst
new file mode 100644 (file)
index 0000000..5a21064
--- /dev/null
@@ -0,0 +1,3 @@
+Fix use-after-free in :meth:`dict.clear` when the dictionary values are
+embedded in an object and a destructor causes re-entrant mutation of the
+dictionary.
index 35ca9933bfa8aed5f56a90839bd8e6e97ad80de2..bcd3c862fd59b25610bf46e2cbd624d02bc5385b 100644 (file)
@@ -2969,6 +2969,21 @@ _PyDict_DelItemIf(PyObject *op, PyObject *key,
     return res;
 }
 
+static void
+clear_embedded_values(PyDictValues *values, Py_ssize_t nentries)
+{
+    PyObject *refs[SHARED_KEYS_MAX_SIZE];
+    assert(nentries <= SHARED_KEYS_MAX_SIZE);
+    for (Py_ssize_t i = 0; i < nentries; i++) {
+        refs[i] = values->values[i];
+        values->values[i] = NULL;
+    }
+    values->size = 0;
+    for (Py_ssize_t i = 0; i < nentries; i++) {
+        Py_XDECREF(refs[i]);
+    }
+}
+
 static void
 clear_lock_held(PyObject *op)
 {
@@ -2997,20 +3012,18 @@ clear_lock_held(PyObject *op)
         assert(oldkeys->dk_refcnt == 1);
         dictkeys_decref(oldkeys, IS_DICT_SHARED(mp));
     }
+    else if (oldvalues->embedded) {
+        clear_embedded_values(oldvalues, oldkeys->dk_nentries);
+    }
     else {
+        set_values(mp, NULL);
+        set_keys(mp, Py_EMPTY_KEYS);
         n = oldkeys->dk_nentries;
         for (i = 0; i < n; i++) {
             Py_CLEAR(oldvalues->values[i]);
         }
-        if (oldvalues->embedded) {
-            oldvalues->size = 0;
-        }
-        else {
-            set_values(mp, NULL);
-            set_keys(mp, Py_EMPTY_KEYS);
-            free_values(oldvalues, IS_DICT_SHARED(mp));
-            dictkeys_decref(oldkeys, false);
-        }
+        free_values(oldvalues, IS_DICT_SHARED(mp));
+        dictkeys_decref(oldkeys, false);
     }
     ASSERT_CONSISTENT(mp);
 }