Skip to content

Commit

Permalink
gh-99127: Allow some features of syslog to the main interpreter only (g…
Browse files Browse the repository at this point in the history
  • Loading branch information
corona10 authored Nov 29, 2022
1 parent ed39109 commit 8bb2303
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 2 deletions.
21 changes: 21 additions & 0 deletions Doc/library/syslog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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]]])

Expand All @@ -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()

Expand All @@ -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)

Expand Down
9 changes: 9 additions & 0 deletions Doc/whatsnew/3.12.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
=============
Expand Down
64 changes: 64 additions & 0 deletions Lib/test/test_syslog.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow some features of :mod:`syslog` to the main interpreter only. Patch by Dong-hee Na.
29 changes: 27 additions & 2 deletions Modules/syslogmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 8bb2303

Please sign in to comment.