Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions Lib/test/test_ordered_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -917,6 +917,40 @@ def __getitem__(self, key):
msg = "OrderedDict mutated during iteration"
self.assertRaisesRegex(RuntimeError, msg, od.copy)

def test_issue142637_clear_in_key_eq(self):
# gh-142637: looking up a key must not crash when its __eq__ clears
# the dict, which replaces the keys being looked up.
class Original:
def __hash__(self):
return 1
class Trigger:
def __hash__(self):
return 1
def __eq__(self, other):
od.clear()
return True

def delitem():
del od[Trigger()]

operations = {
'del': delitem,
'pop': lambda: od.pop(Trigger()),
'move_to_end': lambda: od.move_to_end(Trigger()),
}
for name, operation in operations.items():
with self.subTest(operation=name):
od = self.OrderedDict()
od[Original()] = None
# Like for a dict, the key is not found once the dict is cleared.
self.assertRaises(KeyError, operation)
self.assertEqual(len(od), 0)

od = self.OrderedDict()
od[Original()] = None
od[Trigger()] = 'value'
self.assertEqual(list(od.values()), ['value'])


@unittest.skipUnless(c_coll, 'requires the C version of the collections module')
class CPythonOrderedDictTests(OrderedDictTests,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix a use-after-free in :class:`collections.OrderedDict` when a key's
:meth:`~object.__eq__` method clears the dictionary during a lookup, for
example in ``del od[key]``, :meth:`!pop` and :meth:`!move_to_end`.
31 changes: 20 additions & 11 deletions Objects/odictobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,6 @@ _odict_get_index_raw(PyODictObject *od, PyObject *key, Py_hash_t hash)
{
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od);
PyObject *value = NULL;
PyDictKeysObject *keys = ((PyDictObject *)od)->ma_keys;
Py_ssize_t ix;
#ifdef Py_GIL_DISABLED
ix = _Py_dict_lookup_threadsafe((PyDictObject *)od, key, hash, &value);
Expand All @@ -547,7 +546,9 @@ _odict_get_index_raw(PyODictObject *od, PyObject *key, Py_hash_t hash)
ix = _Py_dict_lookup((PyDictObject *)od, key, hash, &value);
#endif
if (ix == DKIX_EMPTY) {
return keys->dk_nentries; /* index of new entry */
/* A re-entrant __eq__ may have replaced the keys during the lookup,
so they must be read again here. */
return ((PyDictObject *)od)->ma_keys->dk_nentries; /* index of new entry */
}
if (ix < 0)
return -1;
Expand Down Expand Up @@ -602,17 +603,25 @@ _odict_get_index(PyODictObject *od, PyObject *key, Py_hash_t hash)
PyDictKeysObject *keys;

assert(key != NULL);
keys = ((PyDictObject *)od)->ma_keys;
for (;;) {
keys = ((PyDictObject *)od)->ma_keys;

/* Ensure od_fast_nodes and dk_entries are in sync. */
if (od->od_resize_sentinel != keys ||
od->od_fast_nodes_size != (ONE << (keys->dk_log2_size))) {
int resize_res = _odict_resize(od);
if (resize_res < 0)
return -1;
}

/* Ensure od_fast_nodes and dk_entries are in sync. */
if (od->od_resize_sentinel != keys ||
od->od_fast_nodes_size != (ONE << (keys->dk_log2_size))) {
int resize_res = _odict_resize(od);
if (resize_res < 0)
return -1;
Py_ssize_t index = _odict_get_index_raw(od, key, hash);
if (index < 0 || keys == ((PyDictObject *)od)->ma_keys) {
return index;
}
/* The keys were replaced while looking up the key, e.g. by a
re-entrant __eq__ clearing the dict: od_fast_nodes no longer
matches them, so synchronize it and look the key up again. */
}

return _odict_get_index_raw(od, key, hash);
}

/* Returns NULL if there was some error or the key was not found. */
Expand Down
Loading