diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 8a4e19c95a36ef..f28155fe50148d 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -1680,6 +1680,69 @@ def test_hash_collision_remove_add(self): 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 index 00000000000000..5a2106480fb843 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-02-26-12-00-00.gh-issue-130555.TMSOIu.rst @@ -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. diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 35ca9933bfa8ae..bcd3c862fd59b2 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -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); }