[3.14] gh-132835: Add defensive NULL checks to MRO resolution (GH-134763) (GH-140436)
Some checks are pending
Tests / (push) Blocked by required conditions
Tests / Windows MSI (push) Blocked by required conditions
Tests / Check if generated files are up to date (push) Blocked by required conditions
Tests / Change detection (push) Waiting to run
Tests / Docs (push) Blocked by required conditions
Tests / Check if the ABI has changed (push) Blocked by required conditions
Tests / Check if Autoconf files are up to date (push) Blocked by required conditions
Tests / Ubuntu SSL tests with OpenSSL (push) Blocked by required conditions
Tests / Android (aarch64) (push) Blocked by required conditions
Tests / Android (x86_64) (push) Blocked by required conditions
Tests / WASI (push) Blocked by required conditions
Tests / Hypothesis tests on Ubuntu (push) Blocked by required conditions
Tests / Address sanitizer (push) Blocked by required conditions
Tests / Sanitizers (push) Blocked by required conditions
Tests / Cross build Linux (push) Blocked by required conditions
Tests / CIFuzz (push) Blocked by required conditions
Tests / All required checks pass (push) Blocked by required conditions
Lint / lint (push) Waiting to run

Currently, there are a few places where tp_mro could theoretically
become NULL, but do not in practice. This commit adds defensive checks for
NULL values to ensure that any changes do not introduce a crash and that
state invariants are upheld.

The assertions added in this commit are all instances where a NULL value would get passed to something not expecting a NULL, so it is better to catch an assertion failure than crash later on.

There are a few cases where it is OK for the return of lookup_tp_mro to be NULL, such as when passed to is_subtype_with_mro, which handles this explicitly.
(cherry picked from commit a8edca62fc)

Co-authored-by: Emma Smith <emma@emmatyping.dev>
This commit is contained in:
Miss Islington (bot) 2025-10-22 05:45:06 +02:00 committed by GitHub
parent a490d671fa
commit 9d547dab0b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -1658,7 +1658,7 @@ static int recurse_down_subclasses(PyTypeObject *type, PyObject *name,
update_callback callback, void *data); update_callback callback, void *data);
static int static int
mro_hierarchy(PyTypeObject *type, PyObject *temp) mro_hierarchy_for_complete_type(PyTypeObject *type, PyObject *temp)
{ {
ASSERT_TYPE_LOCK_HELD(); ASSERT_TYPE_LOCK_HELD();
@ -1669,6 +1669,7 @@ mro_hierarchy(PyTypeObject *type, PyObject *temp)
return res; return res;
} }
PyObject *new_mro = lookup_tp_mro(type); PyObject *new_mro = lookup_tp_mro(type);
assert(new_mro != NULL);
PyObject *tuple; PyObject *tuple;
if (old_mro != NULL) { if (old_mro != NULL) {
@ -1713,7 +1714,7 @@ mro_hierarchy(PyTypeObject *type, PyObject *temp)
Py_ssize_t n = PyList_GET_SIZE(subclasses); Py_ssize_t n = PyList_GET_SIZE(subclasses);
for (Py_ssize_t i = 0; i < n; i++) { for (Py_ssize_t i = 0; i < n; i++) {
PyTypeObject *subclass = _PyType_CAST(PyList_GET_ITEM(subclasses, i)); PyTypeObject *subclass = _PyType_CAST(PyList_GET_ITEM(subclasses, i));
res = mro_hierarchy(subclass, temp); res = mro_hierarchy_for_complete_type(subclass, temp);
if (res < 0) { if (res < 0) {
break; break;
} }
@ -1795,7 +1796,7 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases)
if (temp == NULL) { if (temp == NULL) {
goto bail; goto bail;
} }
if (mro_hierarchy(type, temp) < 0) { if (mro_hierarchy_for_complete_type(type, temp) < 0) {
goto undo; goto undo;
} }
Py_DECREF(temp); Py_DECREF(temp);
@ -3291,6 +3292,7 @@ mro_implementation_unlocked(PyTypeObject *type)
*/ */
PyTypeObject *base = _PyType_CAST(PyTuple_GET_ITEM(bases, 0)); PyTypeObject *base = _PyType_CAST(PyTuple_GET_ITEM(bases, 0));
PyObject *base_mro = lookup_tp_mro(base); PyObject *base_mro = lookup_tp_mro(base);
assert(base_mro != NULL);
Py_ssize_t k = PyTuple_GET_SIZE(base_mro); Py_ssize_t k = PyTuple_GET_SIZE(base_mro);
PyObject *result = PyTuple_New(k + 1); PyObject *result = PyTuple_New(k + 1);
if (result == NULL) { if (result == NULL) {
@ -3325,9 +3327,12 @@ mro_implementation_unlocked(PyTypeObject *type)
return NULL; return NULL;
} }
PyObject *mro_to_merge;
for (Py_ssize_t i = 0; i < n; i++) { for (Py_ssize_t i = 0; i < n; i++) {
PyTypeObject *base = _PyType_CAST(PyTuple_GET_ITEM(bases, i)); PyTypeObject *base = _PyType_CAST(PyTuple_GET_ITEM(bases, i));
to_merge[i] = lookup_tp_mro(base); mro_to_merge = lookup_tp_mro(base);
assert(mro_to_merge != NULL);
to_merge[i] = mro_to_merge;
} }
to_merge[n] = bases; to_merge[n] = bases;
@ -8623,6 +8628,7 @@ type_ready_inherit(PyTypeObject *type)
// Inherit slots // Inherit slots
PyObject *mro = lookup_tp_mro(type); PyObject *mro = lookup_tp_mro(type);
assert(mro != NULL);
Py_ssize_t n = PyTuple_GET_SIZE(mro); Py_ssize_t n = PyTuple_GET_SIZE(mro);
for (Py_ssize_t i = 1; i < n; i++) { for (Py_ssize_t i = 1; i < n; i++) {
PyObject *b = PyTuple_GET_ITEM(mro, i); PyObject *b = PyTuple_GET_ITEM(mro, i);