From aa7f77edd83ccd20e04e50898ba037d85501b971 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Mon, 24 Jul 2023 11:07:45 -0600 Subject: [PATCH] Merge sudo_module_register_loghandler and sudo_module_set_default_loghandler. We now create the LogHandler class for each interpreter in python_plugin_init() instead of just once in sudo_module_init(). This fixes the crash seen in Py_EndInterpreter() with Python 3.12 and significantly reduces the number of leaked objects tracked by MemorySanitizer. --HG-- branch : 1.9 --- plugins/python/python_loghandler.c | 70 ++++++++++----------------- plugins/python/python_plugin_common.c | 3 +- plugins/python/sudo_python_module.c | 3 -- plugins/python/sudo_python_module.h | 3 -- 4 files changed, 28 insertions(+), 51 deletions(-) diff --git a/plugins/python/python_loghandler.c b/plugins/python/python_loghandler.c index a4bb4a3cb..ee97d5ccf 100644 --- a/plugins/python/python_loghandler.c +++ b/plugins/python/python_loghandler.c @@ -27,8 +27,6 @@ # define PyObject_CallNoArgs(_o) PyObject_CallObject((_o), NULL) #endif -static PyObject *sudo_type_LogHandler; - static void _debug_plugin(int log_level, const char *log_message) { @@ -127,74 +125,58 @@ static PyMethodDef _sudo_LogHandler_class_methods[] = {NULL, NULL, 0, NULL} }; -// This function registers sudo.LogHandler class +// This function creates the sudo.LogHandler class and adds it +// to the root logger. int -sudo_module_register_loghandler(PyObject *py_module) +sudo_module_set_default_loghandler() { debug_decl(sudo_module_register_loghandler, PYTHON_DEBUG_INTERNAL); - PyObject *py_logging_module = NULL, *py_streamhandler = NULL; + PyObject *py_sudo, *py_logging_module = NULL, *py_logger = NULL, + *py_streamhandler = NULL, *py_class = NULL, + *py_loghandler = NULL, *py_result = NULL; + + py_sudo = PyImport_ImportModule("sudo"); + if (py_sudo == NULL) + goto cleanup; py_logging_module = PyImport_ImportModule("logging"); if (py_logging_module == NULL) goto cleanup; + // Get the root logger which all loggers descend from. + py_logger = PyObject_CallMethod(py_logging_module, "getLogger", NULL); + if (py_logger == NULL) + goto cleanup; + py_streamhandler = PyObject_GetAttrString(py_logging_module, "StreamHandler"); if (py_streamhandler == NULL) goto cleanup; - sudo_type_LogHandler = sudo_module_create_class("sudo.LogHandler", + // Create our own handler that is a sub-class of StreamHandler + py_class = sudo_module_create_class("sudo.LogHandler", _sudo_LogHandler_class_methods, py_streamhandler); - if (sudo_type_LogHandler == NULL) + if (py_class == NULL) goto cleanup; - if (PyModule_AddObject(py_module, "LogHandler", sudo_type_LogHandler) < 0) { - Py_CLEAR(sudo_type_LogHandler); + // PyModule_AddObject steals a reference to py_class on success + if (PyModule_AddObject(py_sudo, "LogHandler", py_class) < 0) goto cleanup; - } + Py_INCREF(py_class); - // PyModule_AddObject steals a reference to sudo_type_LogHandler on success - Py_INCREF(sudo_type_LogHandler); - -cleanup: - Py_CLEAR(py_streamhandler); - Py_CLEAR(py_logging_module); - debug_return_int(PyErr_Occurred() ? SUDO_RC_ERROR : SUDO_RC_OK); -} - -// This sets sudo.LogHandler as the default log handler: -// logging.getLogger().addHandler(sudo.LogHandler()) -int -sudo_module_set_default_loghandler(void) -{ - debug_decl(sudo_module_set_default_loghandler, PYTHON_DEBUG_INTERNAL); - - PyObject *py_loghandler = NULL, *py_logging_module = NULL, - *py_logger = NULL, *py_result = NULL; - - py_loghandler = PyObject_CallNoArgs(sudo_type_LogHandler); + py_loghandler = PyObject_CallNoArgs(py_class); if (py_loghandler == NULL) goto cleanup; - py_logging_module = PyImport_ImportModule("logging"); - if (py_logging_module == NULL) - goto cleanup; - - py_logger = PyObject_CallMethod(py_logging_module, "getLogger", NULL); - if (py_logger == NULL) - goto cleanup; - py_result = PyObject_CallMethod(py_logger, "addHandler", "O", py_loghandler); cleanup: Py_CLEAR(py_result); + Py_CLEAR(py_loghandler); + Py_CLEAR(py_class); + Py_CLEAR(py_streamhandler); Py_CLEAR(py_logger); Py_CLEAR(py_logging_module); -#if 0 - // XXX - If we don't leak py_loghandler here we may get a crash in - // Py_EndInterpreter() on Python 3.12. - Py_CLEAR(py_loghandler); -#endif - + Py_CLEAR(py_sudo); debug_return_int(PyErr_Occurred() ? SUDO_RC_ERROR : SUDO_RC_OK); } diff --git a/plugins/python/python_plugin_common.c b/plugins/python/python_plugin_common.c index 508dadc00..ded8b5de5 100644 --- a/plugins/python/python_plugin_common.c +++ b/plugins/python/python_plugin_common.c @@ -532,8 +532,9 @@ python_plugin_init(struct PluginContext *plugin_ctx, char * const plugin_options } PyThreadState_Swap(plugin_ctx->py_interpreter); - if (sudo_module_set_default_loghandler() < 0) + if (sudo_module_set_default_loghandler() != SUDO_RC_OK) { goto cleanup; + } if (_python_plugin_set_path(plugin_ctx, _lookup_value(plugin_options, "ModulePath")) != SUDO_RC_OK) { goto cleanup; diff --git a/plugins/python/sudo_python_module.c b/plugins/python/sudo_python_module.c index 5d7f41910..a700371e3 100644 --- a/plugins/python/sudo_python_module.c +++ b/plugins/python/sudo_python_module.c @@ -595,9 +595,6 @@ sudo_module_init(void) if (sudo_module_register_baseplugin(py_module) != SUDO_RC_OK) goto cleanup; - if (sudo_module_register_loghandler(py_module) != SUDO_RC_OK) - goto cleanup; - cleanup: if (PyErr_Occurred()) { Py_CLEAR(py_module); diff --git a/plugins/python/sudo_python_module.h b/plugins/python/sudo_python_module.h index 6934842b3..7921a9891 100644 --- a/plugins/python/sudo_python_module.h +++ b/plugins/python/sudo_python_module.h @@ -45,9 +45,6 @@ int sudo_module_ConvMessages_to_c(PyObject *py_tuple, Py_ssize_t *num_msgs, stru CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION int sudo_module_register_baseplugin(PyObject *py_module); -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -int sudo_module_register_loghandler(PyObject *py_module); - CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION int sudo_module_set_default_loghandler(void);