Skip to content

Commit 4ecf6ca

Browse files
gh-151818: Fix double-free in _CALL_LIST_APPEND on allocation failure (#151826)
_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.
1 parent 3e13407 commit 4ecf6ca

8 files changed

Lines changed: 133 additions & 7 deletions

File tree

‎Include/internal/pycore_opcode_metadata.h‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Include/internal/pycore_uop_metadata.h‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Lib/test/test_opcache.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import copy
33
import pickle
44
import dis
5+
import sys
56
import threading
67
import types
78
import unittest
@@ -2171,6 +2172,39 @@ class MyList(list): pass
21712172
self.assert_no_opcode(my_list_append, "CALL_LIST_APPEND")
21722173
self.assert_no_opcode(my_list_append, "CALL")
21732174

2175+
@cpython_only
2176+
@requires_specialization
2177+
def test_call_list_append_memory_error(self):
2178+
# gh-151818: if CALL_LIST_APPEND fails to resize the list,
2179+
# the argument must be released exactly once.
2180+
_testcapi = import_module("_testcapi")
2181+
2182+
class Holder:
2183+
pass
2184+
2185+
def list_append(l, holder):
2186+
# holder.item is pushed as a new reference, which
2187+
# CALL_LIST_APPEND steals.
2188+
l.append(holder.item)
2189+
2190+
holder = Holder()
2191+
holder.item = item = object()
2192+
for _ in range(_testinternalcapi.SPECIALIZATION_THRESHOLD):
2193+
list_append([], holder)
2194+
self.assert_specialized(list_append, "CALL_LIST_APPEND")
2195+
2196+
refcount = sys.getrefcount(item)
2197+
# An empty list has no spare capacity, so append() must resize it.
2198+
lst = []
2199+
with self.assertRaises(MemoryError):
2200+
_testcapi.set_nomemory(0)
2201+
try:
2202+
list_append(lst, holder)
2203+
finally:
2204+
_testcapi.remove_mem_hooks()
2205+
self.assertEqual(lst, [])
2206+
self.assertEqual(sys.getrefcount(item), refcount)
2207+
21742208
@cpython_only
21752209
@requires_specialization
21762210
def test_call_c_function_extra_flags(self):
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a double-free crash in ``list.append()`` (the specialized
2+
``CALL_LIST_APPEND`` instruction) when growing the list fails with
3+
:exc:`MemoryError`.

‎Modules/_testinternalcapi/test_cases.c.h‎

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/bytecodes.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5038,7 +5038,10 @@ dummy_func(
50385038
int err = _PyList_AppendTakeRef((PyListObject *)self_o, PyStackRef_AsPyObjectSteal(arg));
50395039
UNLOCK_OBJECT(self_o);
50405040
if (err) {
5041-
ERROR_NO_POP();
5041+
// arg is consumed even on failure: close the other inputs
5042+
// and pop them all, so that arg is not closed a second time.
5043+
DECREF_INPUTS();
5044+
ERROR_IF(true);
50425045
}
50435046
c = callable;
50445047
s = self;

‎Python/executor_cases.c.h‎

Lines changed: 58 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/generated_cases.c.h‎

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)