gh-118331: Fix a couple of issues when list allocation fails (#130811)

* Fix use after free in list objects

Set the items pointer in the list object to NULL after the items array
is freed during list deallocation. Otherwise, we can end up with a list
object added to the free list that contains a pointer to an already-freed
items array.

* Mark `_PyList_FromStackRefStealOnSuccess` as escaping

I think technically it's not escaping, because the only object that
can be decrefed if allocation fails is an exact list, which cannot
execute arbitrary code when it is destroyed. However, this seems less
intrusive than trying to special cases objects in the assert in `_Py_Dealloc`
that checks for non-null stackpointers and shouldn't matter for performance.
This commit is contained in:
mpage 2025-03-05 10:42:09 -08:00 committed by GitHub
parent 2904ec2273
commit d7bb7c7817
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 26 additions and 4 deletions

View File

@ -2028,7 +2028,7 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[266] = {
[BINARY_OP_SUBTRACT_FLOAT] = { true, INSTR_FMT_IXC0000, HAS_EXIT_FLAG | HAS_ERROR_FLAG }, [BINARY_OP_SUBTRACT_FLOAT] = { true, INSTR_FMT_IXC0000, HAS_EXIT_FLAG | HAS_ERROR_FLAG },
[BINARY_OP_SUBTRACT_INT] = { true, INSTR_FMT_IXC0000, HAS_EXIT_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [BINARY_OP_SUBTRACT_INT] = { true, INSTR_FMT_IXC0000, HAS_EXIT_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG },
[BINARY_SLICE] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [BINARY_SLICE] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ESCAPES_FLAG },
[BUILD_LIST] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG }, [BUILD_LIST] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG },
[BUILD_MAP] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [BUILD_MAP] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG },
[BUILD_SET] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [BUILD_SET] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG },
[BUILD_SLICE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG }, [BUILD_SLICE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG },

View File

@ -136,7 +136,7 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = {
[_COPY_FREE_VARS] = HAS_ARG_FLAG, [_COPY_FREE_VARS] = HAS_ARG_FLAG,
[_BUILD_STRING] = HAS_ARG_FLAG | HAS_ERROR_FLAG, [_BUILD_STRING] = HAS_ARG_FLAG | HAS_ERROR_FLAG,
[_BUILD_TUPLE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG, [_BUILD_TUPLE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG,
[_BUILD_LIST] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG, [_BUILD_LIST] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG,
[_LIST_EXTEND] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_LIST_EXTEND] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG,
[_SET_UPDATE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_SET_UPDATE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG,
[_BUILD_SET] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_BUILD_SET] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG,

View File

@ -1,6 +1,9 @@
import sys import sys
from test import list_tests import textwrap
from test import list_tests, support
from test.support import cpython_only from test.support import cpython_only
from test.support.import_helper import import_module
from test.support.script_helper import assert_python_failure
import pickle import pickle
import unittest import unittest
@ -309,5 +312,20 @@ def test_tier2_invalidates_iterator(self):
a.append(4) a.append(4)
self.assertEqual(list(it), []) self.assertEqual(list(it), [])
@support.cpython_only
def test_no_memory(self):
# gh-118331: Make sure we don't crash if list allocation fails
import_module("_testcapi")
code = textwrap.dedent("""
import _testcapi, sys
# Prime the freelist
l = [None]
del l
_testcapi.set_nomemory(0)
l = [None]
""")
_, _, err = assert_python_failure("-c", code)
self.assertIn("MemoryError", err.decode("utf-8"))
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()

View File

@ -533,6 +533,7 @@ list_dealloc(PyObject *self)
Py_XDECREF(op->ob_item[i]); Py_XDECREF(op->ob_item[i]);
} }
free_list_items(op->ob_item, false); free_list_items(op->ob_item, false);
op->ob_item = NULL;
} }
if (PyList_CheckExact(op)) { if (PyList_CheckExact(op)) {
_Py_FREELIST_FREE(lists, op, PyObject_GC_Del); _Py_FREELIST_FREE(lists, op, PyObject_GC_Del);

View File

@ -2545,7 +2545,9 @@
_PyStackRef list; _PyStackRef list;
oparg = CURRENT_OPARG(); oparg = CURRENT_OPARG();
values = &stack_pointer[-oparg]; values = &stack_pointer[-oparg];
_PyFrame_SetStackPointer(frame, stack_pointer);
PyObject *list_o = _PyList_FromStackRefStealOnSuccess(values, oparg); PyObject *list_o = _PyList_FromStackRefStealOnSuccess(values, oparg);
stack_pointer = _PyFrame_GetStackPointer(frame);
if (list_o == NULL) { if (list_o == NULL) {
JUMP_TO_ERROR(); JUMP_TO_ERROR();
} }

View File

@ -1015,7 +1015,9 @@
_PyStackRef *values; _PyStackRef *values;
_PyStackRef list; _PyStackRef list;
values = &stack_pointer[-oparg]; values = &stack_pointer[-oparg];
_PyFrame_SetStackPointer(frame, stack_pointer);
PyObject *list_o = _PyList_FromStackRefStealOnSuccess(values, oparg); PyObject *list_o = _PyList_FromStackRefStealOnSuccess(values, oparg);
stack_pointer = _PyFrame_GetStackPointer(frame);
if (list_o == NULL) { if (list_o == NULL) {
JUMP_TO_LABEL(error); JUMP_TO_LABEL(error);
} }

View File

@ -632,7 +632,6 @@ def has_error_without_pop(op: parser.CodeDef) -> bool:
"_PyGen_GetGeneratorFromFrame", "_PyGen_GetGeneratorFromFrame",
"_PyInterpreterState_GET", "_PyInterpreterState_GET",
"_PyList_AppendTakeRef", "_PyList_AppendTakeRef",
"_PyList_FromStackRefStealOnSuccess",
"_PyList_ITEMS", "_PyList_ITEMS",
"_PyLong_CompactValue", "_PyLong_CompactValue",
"_PyLong_DigitCount", "_PyLong_DigitCount",