From f9e8f9ee8d4968a79ff4fa6f04ff793b1ce24b0c Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Wed, 17 Aug 2022 18:22:37 +0200 Subject: [PATCH 1/9] gh-96046: Check managed dict types have ht_cached_keys --- Objects/typeobject.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index e8c36cf1539911..55e1074a1d8a68 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -6795,6 +6795,13 @@ type_ready_post_checks(PyTypeObject *type) type->tp_name); return -1; } + if (((PyHeapTypeObject*)type)->ht_cached_keys == NULL) { + PyErr_Format(PyExc_SystemError, + "type %s has the Py_TPFLAGS_MANAGED_DICT flag " + "but ht_cached_keys is not initialized.", + type->tp_name); + return -1; + } } else if (type->tp_dictoffset < (Py_ssize_t)sizeof(PyObject)) { if (type->tp_dictoffset + type->tp_basicsize <= 0) { From 1fa05b37f1df034dc2377441742ce0e2ab3ed99d Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Wed, 17 Aug 2022 18:54:21 +0200 Subject: [PATCH 2/9] Move ht_cached_keys initialization into PyType_Ready() --- Objects/typeobject.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 55e1074a1d8a68..141ec89c0e08ba 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3285,11 +3285,6 @@ type_new_impl(type_new_ctx *ctx) // Put the proper slots in place fixup_slot_dispatchers(type); - if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) { - PyHeapTypeObject *et = (PyHeapTypeObject*)type; - et->ht_cached_keys = _PyDict_NewKeysForClass(); - } - if (type_new_set_names(type) < 0) { goto error; } @@ -3834,10 +3829,6 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, goto finally; } - if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) { - res->ht_cached_keys = _PyDict_NewKeysForClass(); - } - if (type->tp_doc) { PyObject *__doc__ = PyUnicode_FromString(_PyType_DocWithoutSignature(type->tp_name, type->tp_doc)); if (!__doc__) { @@ -6772,6 +6763,30 @@ type_ready_set_new(PyTypeObject *type) return 0; } +static int +type_ready_managed_dict(PyTypeObject *type) +{ + if (!(type->tp_flags & Py_TPFLAGS_MANAGED_DICT)) { + return 0; + } + if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE)) { + PyErr_Format(PyExc_SystemError, + "type %s has the Py_TPFLAGS_MANAGED_DICT flag " + "but not Py_TPFLAGS_HEAPTYPE flag.", + type->tp_name); + return -1; + } + PyHeapTypeObject* et = (PyHeapTypeObject*)type; + if (et->ht_cached_keys == NULL) { + et->ht_cached_keys = _PyDict_NewKeysForClass(); + if (et->ht_cached_keys == NULL) { + PyErr_Format(PyExc_SystemError, + "failed to initialize ht_cached_keys of type %s.", + type->tp_name); + return -1; + } + } +} static int type_ready_post_checks(PyTypeObject *type) @@ -6795,13 +6810,6 @@ type_ready_post_checks(PyTypeObject *type) type->tp_name); return -1; } - if (((PyHeapTypeObject*)type)->ht_cached_keys == NULL) { - PyErr_Format(PyExc_SystemError, - "type %s has the Py_TPFLAGS_MANAGED_DICT flag " - "but ht_cached_keys is not initialized.", - type->tp_name); - return -1; - } } else if (type->tp_dictoffset < (Py_ssize_t)sizeof(PyObject)) { if (type->tp_dictoffset + type->tp_basicsize <= 0) { @@ -6857,6 +6865,9 @@ type_ready(PyTypeObject *type) if (type_ready_add_subclasses(type) < 0) { return -1; } + if (type_ready_managed_dict(type) < 0) { + return -1; + } if (type_ready_post_checks(type) < 0) { return -1; } From 4ce0ed2b5a34232f68345c13ca205f393c818936 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Wed, 17 Aug 2022 18:56:40 +0200 Subject: [PATCH 3/9] return 0 --- Objects/typeobject.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 141ec89c0e08ba..7bc1ec781d14e7 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -6786,6 +6786,7 @@ type_ready_managed_dict(PyTypeObject *type) return -1; } } + return 0; } static int From 00af163f05476536700013693e037d3b21af9f60 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Thu, 18 Aug 2022 13:13:38 +0200 Subject: [PATCH 4/9] Move check_basicsize_includes_size_and_offsets --- Objects/typeobject.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 7bc1ec781d14e7..73aeb56d324c35 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3488,11 +3488,11 @@ get_bases_tuple(PyObject *bases_in, PyType_Spec *spec) } static inline int -check_basicsize_includes_size_and_offsets(PyTypeObject* type) +type_check_basicsize_includes_size_and_offsets(PyTypeObject* type) { if (type->tp_alloc != PyType_GenericAlloc) { // Custom allocators can ignore tp_basicsize - return 1; + return 0; } Py_ssize_t max = (Py_ssize_t)type->tp_basicsize; @@ -3501,30 +3501,30 @@ check_basicsize_includes_size_and_offsets(PyTypeObject* type) "tp_basicsize for type '%s' (%d) is too small for base '%s' (%d)", type->tp_name, type->tp_basicsize, type->tp_base->tp_name, type->tp_base->tp_basicsize); - return 0; + return -1; } if (type->tp_weaklistoffset + (Py_ssize_t)sizeof(PyObject*) > max) { PyErr_Format(PyExc_TypeError, "weaklist offset %d is out of bounds for type '%s' (tp_basicsize = %d)", type->tp_weaklistoffset, type->tp_name, type->tp_basicsize); - return 0; + return -1; } if (type->tp_dictoffset + (Py_ssize_t)sizeof(PyObject*) > max) { PyErr_Format(PyExc_TypeError, "dict offset %d is out of bounds for type '%s' (tp_basicsize = %d)", type->tp_dictoffset, type->tp_name, type->tp_basicsize); - return 0; + return -1; } if (type->tp_vectorcall_offset + (Py_ssize_t)sizeof(vectorcallfunc*) > max) { PyErr_Format(PyExc_TypeError, "vectorcall offset %d is out of bounds for type '%s' (tp_basicsize = %d)", type->tp_vectorcall_offset, type->tp_name, type->tp_basicsize); - return 0; + return -1; } - return 1; + return 0; } PyObject * @@ -3825,10 +3825,6 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, goto finally; } - if (!check_basicsize_includes_size_and_offsets(type)) { - goto finally; - } - if (type->tp_doc) { PyObject *__doc__ = PyUnicode_FromString(_PyType_DocWithoutSignature(type->tp_name, type->tp_doc)); if (!__doc__) { @@ -6869,6 +6865,9 @@ type_ready(PyTypeObject *type) if (type_ready_managed_dict(type) < 0) { return -1; } + if (type_check_basicsize_includes_size_and_offsets(type) < 0) { + return -1; + } if (type_ready_post_checks(type) < 0) { return -1; } From d14e57fb790e662fe6c513236c3b68aadbb69f66 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Thu, 18 Aug 2022 13:48:09 +0200 Subject: [PATCH 5/9] Add blurb --- .../2022-08-18-13-47-59.gh-issue-96046.5Hqbka.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-08-18-13-47-59.gh-issue-96046.5Hqbka.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-08-18-13-47-59.gh-issue-96046.5Hqbka.rst b/Misc/NEWS.d/next/Core and Builtins/2022-08-18-13-47-59.gh-issue-96046.5Hqbka.rst new file mode 100644 index 00000000000000..b8cb52d4b4ece9 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-08-18-13-47-59.gh-issue-96046.5Hqbka.rst @@ -0,0 +1,4 @@ +:c:func:`PyType_Ready` now initializes ``ht_cached_keys`` and performs +additional checks to ensure that type objects are properly configured. This +avoids crashes in 3rd party packages that don't use regular API to create +new types. From cfbd154fc47f741ecc6089cbad3770cce6e7ba86 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Thu, 18 Aug 2022 16:43:14 +0200 Subject: [PATCH 6/9] Fix format, use memory error. --- Objects/typeobject.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 73aeb56d324c35..e988644d2592c4 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3505,21 +3505,21 @@ type_check_basicsize_includes_size_and_offsets(PyTypeObject* type) } if (type->tp_weaklistoffset + (Py_ssize_t)sizeof(PyObject*) > max) { PyErr_Format(PyExc_TypeError, - "weaklist offset %d is out of bounds for type '%s' (tp_basicsize = %d)", + "weaklist offset %zd is out of bounds for type '%s' (tp_basicsize = %zd)", type->tp_weaklistoffset, type->tp_name, type->tp_basicsize); return -1; } if (type->tp_dictoffset + (Py_ssize_t)sizeof(PyObject*) > max) { PyErr_Format(PyExc_TypeError, - "dict offset %d is out of bounds for type '%s' (tp_basicsize = %d)", + "dict offset %zd is out of bounds for type '%s' (tp_basicsize = %zd)", type->tp_dictoffset, type->tp_name, type->tp_basicsize); return -1; } if (type->tp_vectorcall_offset + (Py_ssize_t)sizeof(vectorcallfunc*) > max) { PyErr_Format(PyExc_TypeError, - "vectorcall offset %d is out of bounds for type '%s' (tp_basicsize = %d)", + "vectorcall offset %zd is out of bounds for type '%s' (tp_basicsize = %zd)", type->tp_vectorcall_offset, type->tp_name, type->tp_basicsize); return -1; @@ -6768,7 +6768,7 @@ type_ready_managed_dict(PyTypeObject *type) if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE)) { PyErr_Format(PyExc_SystemError, "type %s has the Py_TPFLAGS_MANAGED_DICT flag " - "but not Py_TPFLAGS_HEAPTYPE flag.", + "but not Py_TPFLAGS_HEAPTYPE flag", type->tp_name); return -1; } @@ -6776,9 +6776,7 @@ type_ready_managed_dict(PyTypeObject *type) if (et->ht_cached_keys == NULL) { et->ht_cached_keys = _PyDict_NewKeysForClass(); if (et->ht_cached_keys == NULL) { - PyErr_Format(PyExc_SystemError, - "failed to initialize ht_cached_keys of type %s.", - type->tp_name); + PyErr_NoMemory(); return -1; } } From c301539c76d9d81090f23cc9d2b8a096ccb4b690 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Fri, 19 Aug 2022 12:39:33 +0200 Subject: [PATCH 7/9] Revert "Fix format, use memory error." This reverts commit cfbd154fc47f741ecc6089cbad3770cce6e7ba86. --- Objects/typeobject.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index e988644d2592c4..73aeb56d324c35 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3505,21 +3505,21 @@ type_check_basicsize_includes_size_and_offsets(PyTypeObject* type) } if (type->tp_weaklistoffset + (Py_ssize_t)sizeof(PyObject*) > max) { PyErr_Format(PyExc_TypeError, - "weaklist offset %zd is out of bounds for type '%s' (tp_basicsize = %zd)", + "weaklist offset %d is out of bounds for type '%s' (tp_basicsize = %d)", type->tp_weaklistoffset, type->tp_name, type->tp_basicsize); return -1; } if (type->tp_dictoffset + (Py_ssize_t)sizeof(PyObject*) > max) { PyErr_Format(PyExc_TypeError, - "dict offset %zd is out of bounds for type '%s' (tp_basicsize = %zd)", + "dict offset %d is out of bounds for type '%s' (tp_basicsize = %d)", type->tp_dictoffset, type->tp_name, type->tp_basicsize); return -1; } if (type->tp_vectorcall_offset + (Py_ssize_t)sizeof(vectorcallfunc*) > max) { PyErr_Format(PyExc_TypeError, - "vectorcall offset %zd is out of bounds for type '%s' (tp_basicsize = %zd)", + "vectorcall offset %d is out of bounds for type '%s' (tp_basicsize = %d)", type->tp_vectorcall_offset, type->tp_name, type->tp_basicsize); return -1; @@ -6768,7 +6768,7 @@ type_ready_managed_dict(PyTypeObject *type) if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE)) { PyErr_Format(PyExc_SystemError, "type %s has the Py_TPFLAGS_MANAGED_DICT flag " - "but not Py_TPFLAGS_HEAPTYPE flag", + "but not Py_TPFLAGS_HEAPTYPE flag.", type->tp_name); return -1; } @@ -6776,7 +6776,9 @@ type_ready_managed_dict(PyTypeObject *type) if (et->ht_cached_keys == NULL) { et->ht_cached_keys = _PyDict_NewKeysForClass(); if (et->ht_cached_keys == NULL) { - PyErr_NoMemory(); + PyErr_Format(PyExc_SystemError, + "failed to initialize ht_cached_keys of type %s.", + type->tp_name); return -1; } } From 4f7a95418713b1e25bc34d417d88d2c3073cde20 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Fri, 19 Aug 2022 12:39:43 +0200 Subject: [PATCH 8/9] Revert "Move check_basicsize_includes_size_and_offsets" This reverts commit 00af163f05476536700013693e037d3b21af9f60. --- Objects/typeobject.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 73aeb56d324c35..7bc1ec781d14e7 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3488,11 +3488,11 @@ get_bases_tuple(PyObject *bases_in, PyType_Spec *spec) } static inline int -type_check_basicsize_includes_size_and_offsets(PyTypeObject* type) +check_basicsize_includes_size_and_offsets(PyTypeObject* type) { if (type->tp_alloc != PyType_GenericAlloc) { // Custom allocators can ignore tp_basicsize - return 0; + return 1; } Py_ssize_t max = (Py_ssize_t)type->tp_basicsize; @@ -3501,30 +3501,30 @@ type_check_basicsize_includes_size_and_offsets(PyTypeObject* type) "tp_basicsize for type '%s' (%d) is too small for base '%s' (%d)", type->tp_name, type->tp_basicsize, type->tp_base->tp_name, type->tp_base->tp_basicsize); - return -1; + return 0; } if (type->tp_weaklistoffset + (Py_ssize_t)sizeof(PyObject*) > max) { PyErr_Format(PyExc_TypeError, "weaklist offset %d is out of bounds for type '%s' (tp_basicsize = %d)", type->tp_weaklistoffset, type->tp_name, type->tp_basicsize); - return -1; + return 0; } if (type->tp_dictoffset + (Py_ssize_t)sizeof(PyObject*) > max) { PyErr_Format(PyExc_TypeError, "dict offset %d is out of bounds for type '%s' (tp_basicsize = %d)", type->tp_dictoffset, type->tp_name, type->tp_basicsize); - return -1; + return 0; } if (type->tp_vectorcall_offset + (Py_ssize_t)sizeof(vectorcallfunc*) > max) { PyErr_Format(PyExc_TypeError, "vectorcall offset %d is out of bounds for type '%s' (tp_basicsize = %d)", type->tp_vectorcall_offset, type->tp_name, type->tp_basicsize); - return -1; + return 0; } - return 0; + return 1; } PyObject * @@ -3825,6 +3825,10 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, goto finally; } + if (!check_basicsize_includes_size_and_offsets(type)) { + goto finally; + } + if (type->tp_doc) { PyObject *__doc__ = PyUnicode_FromString(_PyType_DocWithoutSignature(type->tp_name, type->tp_doc)); if (!__doc__) { @@ -6865,9 +6869,6 @@ type_ready(PyTypeObject *type) if (type_ready_managed_dict(type) < 0) { return -1; } - if (type_check_basicsize_includes_size_and_offsets(type) < 0) { - return -1; - } if (type_ready_post_checks(type) < 0) { return -1; } From 0db631aa8cb6a9f0a8c883b43cc873500bffd06b Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Fri, 19 Aug 2022 13:07:44 +0200 Subject: [PATCH 9/9] Use MemoryError --- Objects/typeobject.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 7bc1ec781d14e7..6b2f3491415f71 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -6772,7 +6772,7 @@ type_ready_managed_dict(PyTypeObject *type) if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE)) { PyErr_Format(PyExc_SystemError, "type %s has the Py_TPFLAGS_MANAGED_DICT flag " - "but not Py_TPFLAGS_HEAPTYPE flag.", + "but not Py_TPFLAGS_HEAPTYPE flag", type->tp_name); return -1; } @@ -6780,9 +6780,7 @@ type_ready_managed_dict(PyTypeObject *type) if (et->ht_cached_keys == NULL) { et->ht_cached_keys = _PyDict_NewKeysForClass(); if (et->ht_cached_keys == NULL) { - PyErr_Format(PyExc_SystemError, - "failed to initialize ht_cached_keys of type %s.", - type->tp_name); + PyErr_NoMemory(); return -1; } }