Repository navigation
gh-151818: Fix double-free in _CALL_LIST_APPEND on allocation failure - #151826
Open
timurmamedov1 wants to merge 4 commits into
Open
timurmamedov1 wants to merge 4 commits into
timurmamedov1 wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a reference-management bug in the specialized CALL_LIST_APPEND fast path that could double-close/decref an argument on list-resize allocation failure, leading to a double-free/crash. The change adjusts ownership so the list append consumes an independent reference while the eval-stack PyStackRef remains valid for unwinding.
Changes:
- Update
CALL_LIST_APPENDimplementations to passPy_NewRef(PyStackRef_AsPyObjectBorrow(arg))to_PyList_AppendTakeRef()and explicitlyPyStackRef_CLOSE(arg)only on success. - Adjust stack-pointer synchronization in generated interpreter/test-case code to correctly pop/close
argon the success path. - Simplify Tier 2 uop variants for
_CALL_LIST_APPEND(retain only_CALL_LIST_APPEND_r33) and update uop metadata/IDs accordingly; addHAS_ESCAPES_FLAGfor_CALL_LIST_APPEND.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Python/generated_cases.c.h | Fix ref ownership for _CALL_LIST_APPEND and adjust stack-pointer updates/cleanup to avoid double-close on error. |
| Python/executor_cases.c.h | Fix Tier 2 _CALL_LIST_APPEND_r33 to avoid stealing arg and restore stack correctly on error; remove other variants. |
| Python/bytecodes.c | Update _CALL_LIST_APPEND uop definition to use an independent ref for list append and close arg only on success. |
| Modules/_testinternalcapi/test_cases.c.h | Mirrors interpreter generated-case fix for internal testing harness. |
| Include/internal/pycore_uop_metadata.h | Mark _CALL_LIST_APPEND as escaping and restrict caching configuration to the remaining _r33 variant. |
| Include/internal/pycore_uop_ids.h | Remove _CALL_LIST_APPEND_r03/r13/r23 IDs and renumber subsequent uops. |
| Misc/NEWS.d/next/Core_and_Builtins/2026-06-20-18-00-00.gh-issue-151818.LsApDf.rst | Adds NEWS entry documenting the crash fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5108
to
+5114
| PyObject *arg_o = PyStackRef_AsPyObjectBorrow(arg); | ||
| int err = _PyList_AppendTakeRef((PyListObject *)self_o, Py_NewRef(arg_o)); | ||
| UNLOCK_OBJECT(self_o); | ||
| if (err) { | ||
| ERROR_NO_POP(); | ||
| } | ||
| PyStackRef_CLOSE(arg); |
timurmamedov1
force-pushed
the
gh-151818-fix-list-append-double-free
branch
2 times, most recently
from
June 21, 2026 16:17
c9176ae to
ca1f525
Compare
…ailure _CALL_LIST_APPEND stole the arg stackref via PyStackRef_AsPyObjectSteal and passed it to _PyList_AppendTakeRef. When the list's backing array failed to grow, _PyList_AppendTakeRef decreffed the item, but the stale stackref remained on the value stack. The exception unwinder then closed it a second time, causing a double-free / use-after-free. Fix by giving _PyList_AppendTakeRef a separate reference via Py_NewRef and closing the stackref explicitly on success. On the error path the stackref still holds a valid reference, so the exception unwinder can safely close it.
CALL_LIST_APPEND is not in the Sphinx opcode domain, so use double backticks instead of :opcode: to avoid a broken cross-reference.
timurmamedov1
force-pushed
the
gh-151818-fix-list-append-double-free
branch
from
June 24, 2026 20:00
c63498d to
aa69c52
Compare
|
This PR is stale because it has been open for 90 days with no activity. |
…-list-append-double-free # Conflicts: # Include/internal/pycore_uop_ids.h
Keep stealing the argument instead of taking an extra reference on every append. On failure, close the remaining inputs and pop them all, so that the already released argument is not closed a second time. This leaves the fast path unchanged. Add a regression test. Co-authored-by: Victor Stinner <vstinner@python.org>
Author
github-actions
Bot
removed
the
stale
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_CALL_LIST_APPENDsteals theargstackref viaPyStackRef_AsPyObjectStealand passes it to_PyList_AppendTakeRef. When the list's backing array fails to grow,_PyList_AppendTakeRefdecrefs the item, butERROR_NO_POP()leaves the stale stackref on the value stack, soexception_unwindcloses it a second time. Double-free on debug builds (_Py_NegativeRefcount), segfault on release builds.Fix: On the error path,
DECREF_INPUTS()closes the remaining live inputs (callable,self) andERROR_IF(true)pops all three, so the stolenargis no longer on the stack when the unwinder runs. Same approach as #152114. This restores the error path from before #142711 (which introduced theERROR_NO_POP()) while keeping that change's optimization, and the fast path is unchanged.3.14 is not affected: its
CALL_LIST_APPENDclosesselfandcallablebeforeERROR_IF(err).Test:
test_call_list_append_memory_errorintest_opcacheappends to an empty list (no spare capacity, soappend()has to resize) under_testcapi.set_nomemory()and checks that the argument's refcount is unchanged. Without the fix the refcount drops by one and the debug build then aborts with_Py_NegativeRefcount.Related to gh-151119 / gh-151538 (different bug in the same op - missing eval-stack sync), but this is a distinct bug as noted by @devdanzin.
list.append()when the list grows underMemoryError(_CALL_LIST_APPEND) #151818