Skip to content

gh-151895: Fix marshal.loads() crash on dict reference-tracking failure#151896

Open
tonghuaroot wants to merge 3 commits into
python:mainfrom
tonghuaroot:fix-marshal-dict-rref-null
Open

gh-151895: Fix marshal.loads() crash on dict reference-tracking failure#151896
tonghuaroot wants to merge 3 commits into
python:mainfrom
tonghuaroot:fix-marshal-dict-rref-null

Conversation

@tonghuaroot

@tonghuaroot tonghuaroot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

r_object() in Python/marshal.c could dereference a NULL dictionary. When loading a back-referenced dict (FLAG_REF), the TYPE_DICT branch calls R_REF(v), which expands to v = r_ref(v, flag, p) and can return NULL when r_ref()'s internal PyList_Append() fails under MemoryError. The branch then fell through to PyDict_SetItem(v, ...) with v == NULL, crashing the interpreter (SIGSEGV on a release build, an assertion abort on a debug build) instead of raising a clean MemoryError.

Every sibling container branch — TYPE_TUPLE, TYPE_LIST, TYPE_FROZENDICT, and the set/frozenset branches — already follows R_REF(v) with an if (v == NULL) break; (or if (idx < 0)) guard; TYPE_DICT was the only one missing it. This adds the guard. r_ref() has already decref'd the dict on its failure path, so the early break propagates the exception cleanly and marshal.loads() raises MemoryError.

A regression test is added in Lib/test/test_marshal.py that uses _testcapi.set_nomemory() to fail the reference-tracking allocation while loading a back-referenced dict: it crashes the interpreter without this change and raises MemoryError with it. test_marshal passes.

This is a regression introduced in 2e37d83 (gh-148653), which refactored the TYPE_DICT/TYPE_FROZENDICT branches to share code and dropped the dict path's if (v == NULL) break;. It first shipped in v3.15.0b1, so only main and 3.15 are affected (3.13 and 3.14 still have the guard); no released version is exposed. Needs backport to 3.15.

… failure

Loading a reference-tracked dictionary dereferenced a NULL pointer when
the allocation that registers it for back-references failed under low
memory.  It now raises MemoryError, matching the tuple and list paths.
@aisk

aisk commented Jun 22, 2026

Copy link
Copy Markdown
Member

We usually don't add tests for cases where the return value of malloc is not checked, so I don't think this test case is necessary. Otherwise, the change looks good to me.

Per review: CPython does not usually add tests for unchecked-malloc
failure paths. Keep just the r_object() TYPE_DICT guard.
@tonghuaroot

Copy link
Copy Markdown
Contributor Author

Good point — dropped the test in a178dac; the r_object() TYPE_DICT guard is unchanged. Thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants