[Python-checkins] bpo-32374: m_traverse may be called with m_state=NULL (GH-5140)
Miss Islington (bot)
webhook-mailer at python.org
Sat Mar 17 02:03:59 EDT 2018
https://github.com/python/cpython/commit/136905fffd5f77395f80e3409630c11756b5469c
commit: 136905fffd5f77395f80e3409630c11756b5469c
branch: 3.7
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: GitHub <noreply at github.com>
date: 2018-03-16T23:03:56-07:00
summary:
bpo-32374: m_traverse may be called with m_state=NULL (GH-5140)
Multi-phase initialized modules allow m_traverse to be called while the
module is still being initialized, so module authors may need to account
for that.
(cherry picked from commit c2b0b12d1a137ada1023ab7c10b8d9a0249d95f9)
Co-authored-by: Marcel Plch <gmarcel.plch at gmail.com>
files:
A Misc/NEWS.d/next/C API/2018-01-09-17-03-54.bpo-32374.SwwLoz.rst
M Doc/c-api/module.rst
M Lib/test/test_importlib/extension/test_loader.py
M Modules/_testmultiphase.c
M Objects/moduleobject.c
diff --git a/Doc/c-api/module.rst b/Doc/c-api/module.rst
index 7efab28af724..017b656854a8 100644
--- a/Doc/c-api/module.rst
+++ b/Doc/c-api/module.rst
@@ -196,17 +196,23 @@ or request "multi-phase initialization" by returning the definition struct itsel
.. c:member:: traverseproc m_traverse
A traversal function to call during GC traversal of the module object, or
- *NULL* if not needed.
+ *NULL* if not needed. This function may be called before module state
+ is allocated (:c:func:`PyModule_GetState()` may return `NULL`),
+ and before the :c:member:`Py_mod_exec` function is executed.
.. c:member:: inquiry m_clear
A clear function to call during GC clearing of the module object, or
- *NULL* if not needed.
+ *NULL* if not needed. This function may be called before module state
+ is allocated (:c:func:`PyModule_GetState()` may return `NULL`),
+ and before the :c:member:`Py_mod_exec` function is executed.
.. c:member:: freefunc m_free
A function to call during deallocation of the module object, or *NULL* if
- not needed.
+ not needed. This function may be called before module state
+ is allocated (:c:func:`PyModule_GetState()` may return `NULL`),
+ and before the :c:member:`Py_mod_exec` function is executed.
Single-phase initialization
...........................
diff --git a/Lib/test/test_importlib/extension/test_loader.py b/Lib/test/test_importlib/extension/test_loader.py
index 8d20040aab8d..53ac3c71d4b5 100644
--- a/Lib/test/test_importlib/extension/test_loader.py
+++ b/Lib/test/test_importlib/extension/test_loader.py
@@ -9,7 +9,7 @@
import unittest
import importlib.util
import importlib
-
+from test.support.script_helper import assert_python_failure
class LoaderTests(abc.LoaderTests):
@@ -267,6 +267,20 @@ def test_nonascii(self):
self.assertEqual(module.__name__, name)
self.assertEqual(module.__doc__, "Module named in %s" % lang)
+ @unittest.skipIf(not hasattr(sys, 'gettotalrefcount'),
+ '--with-pydebug has to be enabled for this test')
+ def test_bad_traverse(self):
+ ''' Issue #32374: Test that traverse fails when accessing per-module
+ state before Py_mod_exec was executed.
+ (Multiphase initialization modules only)
+ '''
+ script = """if True:
+ import importlib.util as util
+ spec = util.find_spec('_testmultiphase')
+ spec.name = '_testmultiphase_with_bad_traverse'
+ m = spec.loader.create_module(spec)"""
+ assert_python_failure("-c", script)
+
(Frozen_MultiPhaseExtensionModuleTests,
Source_MultiPhaseExtensionModuleTests
diff --git a/Misc/NEWS.d/next/C API/2018-01-09-17-03-54.bpo-32374.SwwLoz.rst b/Misc/NEWS.d/next/C API/2018-01-09-17-03-54.bpo-32374.SwwLoz.rst
new file mode 100644
index 000000000000..f9cf6d6b99ce
--- /dev/null
+++ b/Misc/NEWS.d/next/C API/2018-01-09-17-03-54.bpo-32374.SwwLoz.rst
@@ -0,0 +1,2 @@
+Document that m_traverse for multi-phase initialized modules can be called
+with m_state=NULL, and add a sanity check
diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c
index 9b04ebf15586..8090a485f4a9 100644
--- a/Modules/_testmultiphase.c
+++ b/Modules/_testmultiphase.c
@@ -10,6 +10,10 @@ typedef struct {
PyObject *x_attr; /* Attributes dictionary */
} ExampleObject;
+typedef struct {
+ PyObject *integer;
+} testmultiphase_state;
+
/* Example methods */
static int
@@ -218,18 +222,21 @@ static int execfunc(PyObject *m)
}
/* Helper for module definitions; there'll be a lot of them */
-#define TEST_MODULE_DEF(name, slots, methods) { \
+
+#define TEST_MODULE_DEF_EX(name, slots, methods, statesize, traversefunc) { \
PyModuleDef_HEAD_INIT, /* m_base */ \
name, /* m_name */ \
PyDoc_STR("Test module " name), /* m_doc */ \
- 0, /* m_size */ \
+ statesize, /* m_size */ \
methods, /* m_methods */ \
slots, /* m_slots */ \
- NULL, /* m_traverse */ \
+ traversefunc, /* m_traverse */ \
NULL, /* m_clear */ \
NULL, /* m_free */ \
}
+#define TEST_MODULE_DEF(name, slots, methods) TEST_MODULE_DEF_EX(name, slots, methods, 0, NULL)
+
PyModuleDef_Slot main_slots[] = {
{Py_mod_exec, execfunc},
{0, NULL},
@@ -613,6 +620,44 @@ PyInit__testmultiphase_exec_unreported_exception(PyObject *spec)
return PyModuleDef_Init(&def_exec_unreported_exception);
}
+static int
+bad_traverse(PyObject *self, visitproc visit, void *arg) {
+ testmultiphase_state *m_state;
+
+ m_state = PyModule_GetState(self);
+ Py_VISIT(m_state->integer);
+ return 0;
+}
+
+static int
+execfunc_with_bad_traverse(PyObject *mod) {
+ testmultiphase_state *m_state;
+
+ m_state = PyModule_GetState(mod);
+ if (m_state == NULL) {
+ return -1;
+ }
+
+ m_state->integer = PyLong_FromLong(0x7fffffff);
+ Py_INCREF(m_state->integer);
+
+ return 0;
+}
+
+static PyModuleDef_Slot slots_with_bad_traverse[] = {
+ {Py_mod_exec, execfunc_with_bad_traverse},
+ {0, NULL}
+};
+
+static PyModuleDef def_with_bad_traverse = TEST_MODULE_DEF_EX(
+ "_testmultiphase_with_bad_traverse", slots_with_bad_traverse, NULL,
+ sizeof(testmultiphase_state), bad_traverse);
+
+PyMODINIT_FUNC
+PyInit__testmultiphase_with_bad_traverse(PyObject *spec) {
+ return PyModuleDef_Init(&def_with_bad_traverse);
+}
+
/*** Helper for imp test ***/
static PyModuleDef imp_dummy_def = TEST_MODULE_DEF("imp_dummy", main_slots, testexport_methods);
@@ -622,3 +667,4 @@ PyInit_imp_dummy(PyObject *spec)
{
return PyModuleDef_Init(&imp_dummy_def);
}
+
diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c
index d6cde4024336..8fb368e41474 100644
--- a/Objects/moduleobject.c
+++ b/Objects/moduleobject.c
@@ -21,6 +21,17 @@ static PyMemberDef module_members[] = {
{0}
};
+
+/* Helper for sanity check for traverse not handling m_state == NULL
+ * Issue #32374 */
+#ifdef Py_DEBUG
+static int
+bad_traverse_test(PyObject *self, void *arg) {
+ assert(self != NULL);
+ return 0;
+}
+#endif
+
PyTypeObject PyModuleDef_Type = {
PyVarObject_HEAD_INIT(&PyType_Type, 0)
"moduledef", /* tp_name */
@@ -345,6 +356,16 @@ PyModule_FromDefAndSpec2(struct PyModuleDef* def, PyObject *spec, int module_api
}
}
+ /* Sanity check for traverse not handling m_state == NULL
+ * This doesn't catch all possible cases, but in many cases it should
+ * make many cases of invalid code crash or raise Valgrind issues
+ * sooner than they would otherwise.
+ * Issue #32374 */
+#ifdef Py_DEBUG
+ if (def->m_traverse != NULL) {
+ def->m_traverse(m, bad_traverse_test, NULL);
+ }
+#endif
Py_DECREF(nameobj);
return m;
More information about the Python-checkins
mailing list