Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.11] gh-95041: Fix several minor issues in syslog.openlog() (GH-95058) #95261

Merged
merged 1 commit into from
Jul 26, 2022
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 <[email protected]>
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
@@ -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);
}
@@ -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);
}


@@ -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;
}
@@ -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;
}

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