Skip to content

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

Merged
serhiy-storchaka merged 3 commits into
python:mainfrom
tonghuaroot:fix-marshal-dict-rref-null
Aug 16, 2026
Merged

gh-151895: Fix marshal.loads() crash on dict reference-tracking failure#151896
serhiy-storchaka merged 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!

@serhiy-storchaka serhiy-storchaka added the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Aug 16, 2026

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👍

@serhiy-storchaka
serhiy-storchaka merged commit 5b96d39 into python:main Aug 16, 2026
64 checks passed
@miss-islington-app

Copy link
Copy Markdown

Thanks @tonghuaroot for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖

@bedevere-app

bedevere-app Bot commented Aug 16, 2026

Copy link
Copy Markdown

GH-155902 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants