From 8bb2303fd75c299d8fc85229889ac75e867c135c Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Wed, 30 Nov 2022 07:58:20 +0900 Subject: [PATCH] gh-99127: Allow some features of syslog to the main interpreter only (gh-99128) --- Doc/library/syslog.rst | 21 ++++++ Doc/whatsnew/3.12.rst | 9 +++ Lib/test/test_syslog.py | 64 +++++++++++++++++++ ...2-11-05-22-26-35.gh-issue-99127.Btk7ih.rst | 1 + Modules/syslogmodule.c | 29 ++++++++- 5 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-11-05-22-26-35.gh-issue-99127.Btk7ih.rst diff --git a/Doc/library/syslog.rst b/Doc/library/syslog.rst index 766ff57cc66d69..f29ef03267b1ba 100644 --- a/Doc/library/syslog.rst +++ b/Doc/library/syslog.rst @@ -40,6 +40,13 @@ The module defines the following functions: it wasn't called prior to the call to :func:`syslog`, deferring to the syslog implementation to call ``openlog()``. + .. versionchanged:: 3.12 + This function is restricted in subinterpreters. + (Only code that runs in multiple interpreters is affected and + the restriction is not relevant for most users.) + :func:`openlog` must be called in the main interpreter before :func:`syslog` may be used + in a subinterpreter. Otherwise it will raise :exc:`RuntimeError`. + .. function:: openlog([ident[, logoption[, facility]]]) @@ -60,6 +67,13 @@ The module defines the following functions: In previous versions, keyword arguments were not allowed, and *ident* was required. + .. versionchanged:: 3.12 + This function is restricted in subinterpreters. + (Only code that runs in multiple interpreters is affected and + the restriction is not relevant for most users.) + This may only be called in the main interpreter. + It will raise :exc:`RuntimeError` if called in a subinterpreter. + .. function:: closelog() @@ -72,6 +86,13 @@ The module defines the following functions: .. audit-event:: syslog.closelog "" syslog.closelog + .. versionchanged:: 3.12 + This function is restricted in subinterpreters. + (Only code that runs in multiple interpreters is affected and + the restriction is not relevant for most users.) + This may only be called in the main interpreter. + It will raise :exc:`RuntimeError` if called in a subinterpreter. + .. function:: setlogmask(maskpri) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index dff4de621b4c49..c0f98b59ccaf0f 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -673,6 +673,15 @@ Changes in the Python API :class:`bytes` type is accepted for bytes strings. (Contributed by Victor Stinner in :gh:`98393`.) +* :func:`syslog.openlog` and :func:`syslog.closelog` now fail if used in subinterpreters. + :func:`syslog.syslog` may still be used in subinterpreters, + but now only if :func:`syslog.openlog` has already been called in the main interpreter. + These new restrictions do not apply to the main interpreter, + so only a very small set of users might be affected. + This change helps with interpreter isolation. Furthermore, :mod:`syslog` is a wrapper + around process-global resources, which are best managed from the main interpreter. + (Contributed by Dong-hee Na in :gh:`99127`.) + Build Changes ============= diff --git a/Lib/test/test_syslog.py b/Lib/test/test_syslog.py index 2125ec58d87e03..54db80fa9df1af 100644 --- a/Lib/test/test_syslog.py +++ b/Lib/test/test_syslog.py @@ -5,6 +5,7 @@ import threading import time import unittest +from textwrap import dedent # XXX(nnorwitz): This test sucks. I don't know of a platform independent way # to verify that the messages were really logged. @@ -78,6 +79,69 @@ def logger(): finally: sys.setswitchinterval(orig_si) + def test_subinterpreter_syslog(self): + # syslog.syslog() is not allowed in subinterpreters, but only if + # syslog.openlog() hasn't been called in the main interpreter yet. + with self.subTest('before openlog()'): + code = dedent(''' + import syslog + caught_error = False + try: + syslog.syslog('foo') + except RuntimeError: + caught_error = True + assert(caught_error) + ''') + res = support.run_in_subinterp(code) + self.assertEqual(res, 0) + + syslog.openlog() + try: + with self.subTest('after openlog()'): + code = dedent(''' + import syslog + syslog.syslog('foo') + ''') + res = support.run_in_subinterp(code) + self.assertEqual(res, 0) + finally: + syslog.closelog() + + def test_subinterpreter_openlog(self): + try: + code = dedent(''' + import syslog + caught_error = False + try: + syslog.openlog() + except RuntimeError: + caught_error = True + + assert(caught_error) + ''') + res = support.run_in_subinterp(code) + self.assertEqual(res, 0) + finally: + syslog.closelog() + + def test_subinterpreter_closelog(self): + syslog.openlog('python') + try: + code = dedent(''' + import syslog + caught_error = False + try: + syslog.closelog() + except RuntimeError: + caught_error = True + + assert(caught_error) + ''') + res = support.run_in_subinterp(code) + self.assertEqual(res, 0) + finally: + syslog.closelog() + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-05-22-26-35.gh-issue-99127.Btk7ih.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-05-22-26-35.gh-issue-99127.Btk7ih.rst new file mode 100644 index 00000000000000..e93ae4e7b127d1 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-11-05-22-26-35.gh-issue-99127.Btk7ih.rst @@ -0,0 +1 @@ +Allow some features of :mod:`syslog` to the main interpreter only. Patch by Dong-hee Na. diff --git a/Modules/syslogmodule.c b/Modules/syslogmodule.c index 23833b72850313..f45aa5227f1cbf 100644 --- a/Modules/syslogmodule.c +++ b/Modules/syslogmodule.c @@ -61,10 +61,16 @@ module syslog #include "clinic/syslogmodule.c.h" -/* only one instance, only one syslog, so globals should be ok */ -static PyObject *S_ident_o = NULL; /* identifier, held by openlog() */ +/* only one instance, only one syslog, so globals should be ok, + * these fields are writable from the main interpreter only. */ +static PyObject *S_ident_o = NULL; // identifier, held by openlog() static char S_log_open = 0; +static inline int +is_main_interpreter(void) +{ + return (PyInterpreterState_Get() == PyInterpreterState_Main()); +} static PyObject * syslog_get_argv(void) @@ -135,6 +141,13 @@ syslog_openlog_impl(PyObject *module, PyObject *ident, long logopt, long facility) /*[clinic end generated code: output=5476c12829b6eb75 input=8a987a96a586eee7]*/ { + // Since the sys.openlog changes the process level state of syslog library, + // this operation is only allowed for the main interpreter. + if (!is_main_interpreter()) { + PyErr_SetString(PyExc_RuntimeError, "subinterpreter can't use syslog.openlog()"); + return NULL; + } + const char *ident_str = NULL; if (ident) { @@ -195,6 +208,11 @@ syslog_syslog_impl(PyObject *module, int group_left_1, int priority, /* if log is not opened, open it now */ if (!S_log_open) { + if (!is_main_interpreter()) { + PyErr_SetString(PyExc_RuntimeError, "subinterpreter can't use syslog.syslog() " + "until the syslog is opened by the main interpreter"); + return NULL; + } PyObject *openlog_ret = syslog_openlog_impl(module, NULL, 0, LOG_USER); if (openlog_ret == NULL) { return NULL; @@ -229,6 +247,13 @@ static PyObject * syslog_closelog_impl(PyObject *module) /*[clinic end generated code: output=97890a80a24b1b84 input=fb77a54d447acf07]*/ { + // Since the sys.closelog changes the process level state of syslog library, + // this operation is only allowed for the main interpreter. + if (!is_main_interpreter()) { + PyErr_SetString(PyExc_RuntimeError, "sunbinterpreter can't use syslog.closelog()"); + return NULL; + } + if (PySys_Audit("syslog.closelog", NULL) < 0) { return NULL; }