Skip to content

Commit 6f86288

Browse files
[3.14] gh-148660: Fix use-after-free in OrderedDict.copy() on reentrant mutation (GH-151573) (#152541)
gh-148660: Fix use-after-free in OrderedDict.copy() on reentrant mutation (GH-151573) * gh-148660: Fix use-after-free in OrderedDict.copy() on reentrant mutation OrderedDict.copy() walks the internal linked list while building the new dict. The loop body can run arbitrary Python (a key's __eq__/__hash__, or a subclass __getitem__/__setitem__) which can clear the source dict and free the nodes being iterated. Detect this the same way OrderedDict.__eq__ already does (gh-119004): snapshot od_state before the loop, hold a strong reference to the key and read the hash before any reentrant call, and raise RuntimeError if the state changed before advancing to the next node. * gh-148660: fix NEWS nit, suppress undocumented OrderedDict.copy xref (cherry picked from commit 7d128e3) Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
1 parent 11a2482 commit 6f86288

3 files changed

Lines changed: 62 additions & 10 deletions

File tree

Lib/test/test_ordered_dict.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -873,6 +873,39 @@ def side_effect(self):
873873
self.assertDictEqual(dict1, dict.fromkeys((0, 4.2)))
874874
self.assertDictEqual(dict2, dict.fromkeys((0, Key(), 4.2)))
875875

876+
def test_issue148660_copy_clear_in_key_eq(self):
877+
# gh-148660: od.copy() must not crash when a key's __eq__ clears od
878+
# while copy() is inserting into the new dict.
879+
armed = False
880+
calls = 0
881+
class Key:
882+
def __hash__(self):
883+
return 1
884+
def __eq__(self, other):
885+
nonlocal calls
886+
if armed:
887+
calls += 1
888+
if calls == 2:
889+
od.clear()
890+
return self is other
891+
od = self.OrderedDict()
892+
od[Key()] = "v1"
893+
od[Key()] = "v2"
894+
armed = True
895+
msg = "OrderedDict mutated during iteration"
896+
self.assertRaisesRegex(RuntimeError, msg, od.copy)
897+
898+
def test_issue148660_copy_clear_in_subclass_getitem(self):
899+
# gh-148660: od.copy() must not crash when a subclass __getitem__
900+
# clears od.
901+
class OD(self.OrderedDict):
902+
def __getitem__(self, key):
903+
od.clear()
904+
return "v"
905+
od = OD([(1, "v1"), (2, "v2")])
906+
msg = "OrderedDict mutated during iteration"
907+
self.assertRaisesRegex(RuntimeError, msg, od.copy)
908+
876909

877910
@unittest.skipUnless(c_coll, 'requires the C version of the collections module')
878911
class CPythonOrderedDictTests(OrderedDictTests,
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a crash in :meth:`!collections.OrderedDict.copy` when a key's
2+
``__eq__`` or a subclass method mutates the dict during the copy. Now
3+
raises :exc:`RuntimeError` instead, as iteration does.

Objects/odictobject.c

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,36 +1253,52 @@ OrderedDict_copy_impl(PyObject *od)
12531253
if (od_copy == NULL)
12541254
return NULL;
12551255

1256+
/* The loop body may run arbitrary Python code which could mutate od and
1257+
free its nodes (gh-148660); detect that the same way __eq__ does. */
1258+
size_t state = _PyODictObject_CAST(od)->od_state;
1259+
12561260
if (PyODict_CheckExact(od)) {
12571261
_odict_FOREACH(od, node) {
1258-
PyObject *key = _odictnode_KEY(node);
1259-
PyObject *value = _odictnode_VALUE(node, od);
1262+
PyObject *key = Py_NewRef(_odictnode_KEY(node));
1263+
Py_hash_t hash = _odictnode_HASH(node);
1264+
PyObject *value = PyODict_GetItemWithError(od, key);
12601265
if (value == NULL) {
12611266
if (!PyErr_Occurred())
12621267
PyErr_SetObject(PyExc_KeyError, key);
1268+
Py_DECREF(key);
12631269
goto fail;
12641270
}
1265-
if (_PyODict_SetItem_KnownHash_LockHeld((PyObject *)od_copy, key, value,
1266-
_odictnode_HASH(node)) != 0)
1271+
int res = _PyODict_SetItem_KnownHash_LockHeld((PyObject *)od_copy,
1272+
key, value, hash);
1273+
Py_DECREF(key);
1274+
if (res != 0)
12671275
goto fail;
1276+
if (_PyODictObject_CAST(od)->od_state != state)
1277+
goto mutated;
12681278
}
12691279
}
12701280
else {
12711281
_odict_FOREACH(od, node) {
1272-
int res;
1273-
PyObject *value = PyObject_GetItem((PyObject *)od,
1274-
_odictnode_KEY(node));
1275-
if (value == NULL)
1282+
PyObject *key = Py_NewRef(_odictnode_KEY(node));
1283+
PyObject *value = PyObject_GetItem(od, key);
1284+
if (value == NULL) {
1285+
Py_DECREF(key);
12761286
goto fail;
1277-
res = PyObject_SetItem((PyObject *)od_copy,
1278-
_odictnode_KEY(node), value);
1287+
}
1288+
int res = PyObject_SetItem((PyObject *)od_copy, key, value);
12791289
Py_DECREF(value);
1290+
Py_DECREF(key);
12801291
if (res != 0)
12811292
goto fail;
1293+
if (_PyODictObject_CAST(od)->od_state != state)
1294+
goto mutated;
12821295
}
12831296
}
12841297
return od_copy;
12851298

1299+
mutated:
1300+
PyErr_SetString(PyExc_RuntimeError,
1301+
"OrderedDict mutated during iteration");
12861302
fail:
12871303
Py_DECREF(od_copy);
12881304
return NULL;

0 commit comments

Comments
 (0)