]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-118331: Fix a couple of issues when list allocation fails (#130811)
authormpage <mpage@meta.com>
Wed, 5 Mar 2025 18:42:09 +0000 (10:42 -0800)
committerGitHub <noreply@github.com>
Wed, 5 Mar 2025 18:42:09 +0000 (10:42 -0800)
* 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.

Include/internal/pycore_opcode_metadata.h
Include/internal/pycore_uop_metadata.h
Lib/test/test_list.py
Objects/listobject.c
Python/executor_cases.c.h
Python/generated_cases.c.h
Tools/cases_generator/analyzer.py

index b6d85490eef1f3479b72dd044d6d426c596c4ce5..4aae1ffc350dfe12b742f860553ac5734bd1c257 100644 (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_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 },
-    [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_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 },
index 0013540c4969387d37653826f0c98c079c1fe341..fe4857e827478cf876993ccf414df025229ed61e 100644 (file)
@@ -136,7 +136,7 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = {
     [_COPY_FREE_VARS] = HAS_ARG_FLAG,
     [_BUILD_STRING] = HAS_ARG_FLAG | HAS_ERROR_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,
     [_SET_UPDATE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG,
     [_BUILD_SET] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG,
index ad7accf2099f43dac2e0a2d8fb1eaf91954835cb..2a34fd04f35059d796e5de2cd1636ac9598d38d7 100644 (file)
@@ -1,6 +1,9 @@
 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_helper import import_module
+from test.support.script_helper import assert_python_failure
 import pickle
 import unittest
 
@@ -309,5 +312,20 @@ class ListTest(list_tests.CommonTest):
             a.append(4)
             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__":
     unittest.main()
index 84faa5a32a1f2aa019942569ad2bb6c08fdafd7a..2893acf6d6e143fd7821c9e0f0923170084e6242 100644 (file)
@@ -533,6 +533,7 @@ list_dealloc(PyObject *self)
             Py_XDECREF(op->ob_item[i]);
         }
         free_list_items(op->ob_item, false);
+        op->ob_item = NULL;
     }
     if (PyList_CheckExact(op)) {
         _Py_FREELIST_FREE(lists, op, PyObject_GC_Del);
index e164f11620de41dec831d4a85cc387785a249fec..29160b9f6634c54bccbd2f2d7bdbcd7d45b4c786 100644 (file)
             _PyStackRef list;
             oparg = CURRENT_OPARG();
             values = &stack_pointer[-oparg];
+            _PyFrame_SetStackPointer(frame, stack_pointer);
             PyObject *list_o = _PyList_FromStackRefStealOnSuccess(values, oparg);
+            stack_pointer = _PyFrame_GetStackPointer(frame);
             if (list_o == NULL) {
                 JUMP_TO_ERROR();
             }
index 8c3c0e3910b8d15d5eaec7d99a20e4e61bbd71d8..5216918560a48783e3017a428aa4f4908165c89f 100644 (file)
             _PyStackRef *values;
             _PyStackRef list;
             values = &stack_pointer[-oparg];
+            _PyFrame_SetStackPointer(frame, stack_pointer);
             PyObject *list_o = _PyList_FromStackRefStealOnSuccess(values, oparg);
+            stack_pointer = _PyFrame_GetStackPointer(frame);
             if (list_o == NULL) {
                 JUMP_TO_LABEL(error);
             }
index 162a0fdb2cc459494dd76b67e8734b3c96d0c13c..cecfb6f3834d4465e4b550bf12f36ce7cda71df6 100644 (file)
@@ -632,7 +632,6 @@ NON_ESCAPING_FUNCTIONS = (
     "_PyGen_GetGeneratorFromFrame",
     "_PyInterpreterState_GET",
     "_PyList_AppendTakeRef",
-    "_PyList_FromStackRefStealOnSuccess",
     "_PyList_ITEMS",
     "_PyLong_CompactValue",
     "_PyLong_DigitCount",