Visitar URL original
[3.11] gh-95041: Fix several minor issues in syslog.openlog() (GH-95058) by miss-islington · Pull Request #95261 · python/cpython · GitHub
Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
gh-95041: Fix several minor issues in syslog.openlog() (GH-95058)
* syslog_get_argv() swallows exceptions, but not in all cases.
* if ident is non UTF-8 encodable, syslog.openlog() fails after setting the
  global reference to ident. Now the C string saved internally in the previous
  call to openlog() points to the freed memory.
* PySys_Audit() can crash if ident is NULL.
* There may be a race condition with syslog.syslog(), because the global
  reference to ident is decrefed before setting the new value.
* Possible use of freed memory if syslog.openlog() is called while
  the GIL is released in syslog.syslog().
(cherry picked from commit 68c555a)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
  • Loading branch information
serhiy-storchaka authored and miss-islington committed Jul 26, 2022
commit 9173e7b0870d2c4d438a27a0d5fa5c4f194f50e3
57 changes: 33 additions & 24 deletions Modules/syslogmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ syslog_get_argv(void)
}

scriptobj = PyList_GetItem(argv, 0);
if (scriptobj == NULL) {
PyErr_Clear();
return NULL;
}
if (!PyUnicode_Check(scriptobj)) {
return(NULL);
}
Expand All @@ -96,16 +100,16 @@ syslog_get_argv(void)
}

slash = PyUnicode_FindChar(scriptobj, SEP, 0, scriptlen, -1);
if (slash == -2)
if (slash == -2) {
PyErr_Clear();
return NULL;
}
if (slash != -1) {
return PyUnicode_Substring(scriptobj, slash + 1, scriptlen);
} else {
Py_INCREF(scriptobj);
return(scriptobj);
}

return(NULL);
}


Expand All @@ -114,42 +118,41 @@ syslog_openlog(PyObject * self, PyObject * args, PyObject *kwds)
{
long logopt = 0;
long facility = LOG_USER;
PyObject *new_S_ident_o = NULL;
PyObject *ident = NULL;
static char *keywords[] = {"ident", "logoption", "facility", 0};
const char *ident = NULL;
const char *ident_str = NULL;

if (!PyArg_ParseTupleAndKeywords(args, kwds,
"|Ull:openlog", keywords, &new_S_ident_o, &logopt, &facility))
"|Ull:openlog", keywords, &ident, &logopt, &facility))
return NULL;

if (new_S_ident_o) {
Py_INCREF(new_S_ident_o);
if (ident) {
Py_INCREF(ident);
}

/* get sys.argv[0] or NULL if we can't for some reason */
if (!new_S_ident_o) {
new_S_ident_o = syslog_get_argv();
else {
/* get sys.argv[0] or NULL if we can't for some reason */
ident = syslog_get_argv();
}

Py_XDECREF(S_ident_o);
S_ident_o = new_S_ident_o;

/* At this point, S_ident_o should be INCREF()ed. openlog(3) does not
* make a copy, and syslog(3) later uses it. We can't garbagecollect it
/* At this point, ident should be INCREF()ed. openlog(3) does not
* make a copy, and syslog(3) later uses it. We can't garbagecollect it.
* If NULL, just let openlog figure it out (probably using C argv[0]).
*/
if (S_ident_o) {
ident = PyUnicode_AsUTF8(S_ident_o);
if (ident == NULL)
if (ident) {
ident_str = PyUnicode_AsUTF8(ident);
if (ident_str == NULL) {
Py_DECREF(ident);
return NULL;
}
}

if (PySys_Audit("syslog.openlog", "sll", ident, logopt, facility) < 0) {
if (PySys_Audit("syslog.openlog", "Oll", ident ? ident : Py_None, logopt, facility) < 0) {
Py_DECREF(ident);
return NULL;
}

openlog(ident, logopt, facility);
openlog(ident_str, logopt, facility);
S_log_open = 1;
Py_XSETREF(S_ident_o, ident);

Py_RETURN_NONE;
}
Expand Down Expand Up @@ -193,9 +196,15 @@ syslog_syslog(PyObject * self, PyObject * args)
}
}

/* Incref ident, because it can be decrefed if syslog.openlog() is
* called when the GIL is released.
*/
PyObject *ident = S_ident_o;
Py_XINCREF(ident);
Py_BEGIN_ALLOW_THREADS;
syslog(priority, "%s", message);
Py_END_ALLOW_THREADS;
Py_XDECREF(ident);
Py_RETURN_NONE;
}

Expand Down Expand Up @@ -355,4 +364,4 @@ PyMODINIT_FUNC
PyInit_syslog(void)
{
return PyModuleDef_Init(&syslogmodule);
}
}