Stowage/cpython
2
0
Fork 0
mirror of https://github.com/python/cpython.git synced 2025-11-04 07:31:38 +00:00
Code Issues Projects Releases Packages Wiki Activity

bpo-20104: Fix leaks and errors in new os.posix_spawn (GH-5418)

Browse source 
* Fix memory leaks and error handling in posix spawn
* Improve error handling when destroying the file_actions object
* Py_DECREF the result of PySequence_Fast on error
* Handle uninitialized pid
* Use OSError if file actions fails to initialize
* Move _file_actions to outer scope to avoid undefined behaviour
* Remove HAVE_POSIX_SPAWN define in Modules/posixmodule.c
* Unshadow exception and clean error message
This commit is contained in:
Pablo Galindo 2018-01-29 20:34:42 +00:00 • committed by Gregory P. Smith
parent c65ef772c3
commit 0cd6bca655
1 changed files with 66 additions and 52 deletions
Download patch file Download diff file Expand all files Collapse all files

112
Modules/posixmodule.c
View file

@ -176,14 +176,6 @@ corresponding Unix manual entries for more information on calls.");
#else #else
/* Unix functions that the configure script doesn't check for */ /* Unix functions that the configure script doesn't check for */
#define HAVE_EXECV 1 #define HAVE_EXECV 1
/* bpo-32705: Current Android does not have posix_spawn
* Most likely posix_spawn will be available in next Android version (Android
* P, API 28). Need revisit then. See
* https://android-review.googlesource.com/c/platform/bionic/+/504842
**/
#ifndef __ANDROID__
#define HAVE_POSIX_SPAWN 1
#endif
#define HAVE_FORK 1 #define HAVE_FORK 1
#if defined(__USLC__) && defined(__SCO_VERSION__) /* SCO UDK Compiler */ #if defined(__USLC__) && defined(__SCO_VERSION__) /* SCO UDK Compiler */
#define HAVE_FORK1 1 #define HAVE_FORK1 1
@ -5139,8 +5131,13 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
/*[clinic end generated code: output=d023521f541c709c input=0ec9f1cfdc890be5]*/ /*[clinic end generated code: output=d023521f541c709c input=0ec9f1cfdc890be5]*/
{ {
EXECV_CHAR **argvlist = NULL; EXECV_CHAR **argvlist = NULL;
EXECV_CHAR **envlist; EXECV_CHAR **envlist = NULL;
posix_spawn_file_actions_t _file_actions;
posix_spawn_file_actions_t *file_actionsp = NULL;
Py_ssize_t argc, envc; Py_ssize_t argc, envc;
PyObject* result = NULL;
PyObject* seq = NULL;
/* posix_spawn has three arguments: (path, argv, env), where /* posix_spawn has three arguments: (path, argv, env), where
argv is a list or tuple of strings and env is a dictionary argv is a list or tuple of strings and env is a dictionary
@ -5149,7 +5146,7 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
if (!PySequence_Check(argv)) { if (!PySequence_Check(argv)) {
PyErr_SetString(PyExc_TypeError, PyErr_SetString(PyExc_TypeError,
"posix_spawn: argv must be a tuple or list"); "posix_spawn: argv must be a tuple or list");
goto fail; goto exit;
} }
argc = PySequence_Size(argv); argc = PySequence_Size(argv);
if (argc < 1) { if (argc < 1) {
@ -5160,40 +5157,37 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
if (!PyMapping_Check(env)) { if (!PyMapping_Check(env)) {
PyErr_SetString(PyExc_TypeError, PyErr_SetString(PyExc_TypeError,
"posix_spawn: environment must be a mapping object"); "posix_spawn: environment must be a mapping object");
goto fail; goto exit;
} }
argvlist = parse_arglist(argv, &argc); argvlist = parse_arglist(argv, &argc);
if (argvlist == NULL) { if (argvlist == NULL) {
goto fail; goto exit;
} }
if (!argvlist[0][0]) { if (!argvlist[0][0]) {
PyErr_SetString(PyExc_ValueError, PyErr_SetString(PyExc_ValueError,
"posix_spawn: argv first element cannot be empty"); "posix_spawn: argv first element cannot be empty");
goto fail; goto exit;
} }
envlist = parse_envlist(env, &envc); envlist = parse_envlist(env, &envc);
if (envlist == NULL) if (envlist == NULL) {
goto fail; goto exit;
pid_t pid;
posix_spawn_file_actions_t *file_actionsp = NULL;
if (file_actions != NULL && file_actions != Py_None){
posix_spawn_file_actions_t _file_actions;
if(posix_spawn_file_actions_init(&_file_actions) != 0){
PyErr_SetString(PyExc_TypeError,
"Error initializing file actions");
goto fail;
} }
pid_t pid;
if (file_actions != NULL && file_actions != Py_None) {
if(posix_spawn_file_actions_init(&_file_actions) != 0){
PyErr_SetString(PyExc_OSError,
"Error initializing file actions");
goto exit;
}
file_actionsp = &_file_actions; file_actionsp = &_file_actions;
seq = PySequence_Fast(file_actions, "file_actions must be a sequence");
PyObject* seq = PySequence_Fast(file_actions, "file_actions must be a sequence");
if(seq == NULL) { if(seq == NULL) {
goto fail; goto exit;
} }
PyObject* file_actions_obj; PyObject* file_actions_obj;
PyObject* mode_obj; PyObject* mode_obj;
@ -5203,7 +5197,7 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
if(!PySequence_Check(file_actions_obj) | !PySequence_Size(file_actions_obj)) { if(!PySequence_Check(file_actions_obj) | !PySequence_Size(file_actions_obj)) {
PyErr_SetString(PyExc_TypeError,"Each file_action element must be a non empty sequence"); PyErr_SetString(PyExc_TypeError,"Each file_action element must be a non empty sequence");
goto fail; goto exit;
} }
@ -5217,81 +5211,101 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
case POSIX_SPAWN_OPEN: case POSIX_SPAWN_OPEN:
if(PySequence_Size(file_actions_obj) != 5) { if(PySequence_Size(file_actions_obj) != 5) {
PyErr_SetString(PyExc_TypeError,"A open file_action object must have 5 elements"); PyErr_SetString(PyExc_TypeError,"A open file_action object must have 5 elements");
goto fail; goto exit;
} }
long open_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1)); long open_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));
if(PyErr_Occurred()) { if(PyErr_Occurred()) {
goto fail; goto exit;
} }
const char* open_path = PyUnicode_AsUTF8(PySequence_GetItem(file_actions_obj, 2)); const char* open_path = PyUnicode_AsUTF8(PySequence_GetItem(file_actions_obj, 2));
if(open_path == NULL) { if(open_path == NULL) {
goto fail; goto exit;
} }
long open_oflag = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 3)); long open_oflag = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 3));
if(PyErr_Occurred()) { if(PyErr_Occurred()) {
goto fail; goto exit;
} }
long open_mode = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 4)); long open_mode = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 4));
if(PyErr_Occurred()) { if(PyErr_Occurred()) {
goto fail; goto exit;
} }
posix_spawn_file_actions_addopen(file_actionsp, open_fd, open_path, open_oflag, open_mode); if(posix_spawn_file_actions_addopen(file_actionsp, open_fd, open_path, open_oflag, open_mode)) {
PyErr_SetString(PyExc_OSError,"Failed to add open file action");
goto exit;
}
break; break;
case POSIX_SPAWN_CLOSE: case POSIX_SPAWN_CLOSE:
if(PySequence_Size(file_actions_obj) != 2){ if(PySequence_Size(file_actions_obj) != 2){
PyErr_SetString(PyExc_TypeError,"A close file_action object must have 2 elements"); PyErr_SetString(PyExc_TypeError,"A close file_action object must have 2 elements");
goto fail; goto exit;
} }
long close_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1)); long close_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));
if(PyErr_Occurred()) { if(PyErr_Occurred()) {
goto fail; goto exit;
}
if(posix_spawn_file_actions_addclose(file_actionsp, close_fd)) {
PyErr_SetString(PyExc_OSError,"Failed to add close file action");
goto exit;
} }
posix_spawn_file_actions_addclose(file_actionsp, close_fd);
break; break;
case POSIX_SPAWN_DUP2: case POSIX_SPAWN_DUP2:
if(PySequence_Size(file_actions_obj) != 3){ if(PySequence_Size(file_actions_obj) != 3){
PyErr_SetString(PyExc_TypeError,"A dup2 file_action object must have 3 elements"); PyErr_SetString(PyExc_TypeError,"A dup2 file_action object must have 3 elements");
goto fail; goto exit;
} }
long fd1 = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1)); long fd1 = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));
if(PyErr_Occurred()) { if(PyErr_Occurred()) {
goto fail; goto exit;
} }
long fd2 = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 2)); long fd2 = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 2));
if(PyErr_Occurred()) { if(PyErr_Occurred()) {
goto fail; goto exit;
}
if(posix_spawn_file_actions_adddup2(file_actionsp, fd1, fd2)) {
PyErr_SetString(PyExc_OSError,"Failed to add dup2 file action");
goto exit;
} }
posix_spawn_file_actions_adddup2(file_actionsp, fd1, fd2);
break; break;
default: default:
PyErr_SetString(PyExc_TypeError,"Unknown file_actions identifier"); PyErr_SetString(PyExc_TypeError,"Unknown file_actions identifier");
goto fail; goto exit;
} }
} }
Py_DECREF(seq);
} }
_Py_BEGIN_SUPPRESS_IPH _Py_BEGIN_SUPPRESS_IPH
posix_spawn(&pid, path->narrow, file_actionsp, NULL, argvlist, envlist); int err_code = posix_spawn(&pid, path->narrow, file_actionsp, NULL, argvlist, envlist);
return PyLong_FromPid(pid);
_Py_END_SUPPRESS_IPH _Py_END_SUPPRESS_IPH
if(err_code) {
PyErr_SetString(PyExc_OSError,"posix_spawn call failed");
goto exit;
}
result = PyLong_FromPid(pid);
path_error(path); exit:
Py_XDECREF(seq);
if(file_actionsp) {
posix_spawn_file_actions_destroy(file_actionsp);
}
if (envlist) {
free_string_array(envlist, envc); free_string_array(envlist, envc);
}
fail:
if (argvlist) { if (argvlist) {
free_string_array(argvlist, argc); free_string_array(argvlist, argc);
} }
return NULL;
return result;
} }