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.12] gh-104522: Fix OSError raised when run a subprocess (GH-114195) #114219

Merged
merged 1 commit into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 8 additions & 3 deletions Lib/subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -1938,16 +1938,21 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
SubprocessError)
if issubclass(child_exception_type, OSError) and hex_errno:
errno_num = int(hex_errno, 16)
child_exec_never_called = (err_msg == "noexec")
if child_exec_never_called:
if err_msg == "noexec:chdir":
err_msg = ""
# The error must be from chdir(cwd).
err_filename = cwd
elif err_msg == "noexec":
err_msg = ""
err_filename = None
else:
err_filename = orig_executable
if errno_num != 0:
err_msg = os.strerror(errno_num)
raise child_exception_type(errno_num, err_msg, err_filename)
if err_filename is not None:
raise child_exception_type(errno_num, err_msg, err_filename)
else:
raise child_exception_type(errno_num, err_msg)
raise child_exception_type(err_msg)


Expand Down
12 changes: 7 additions & 5 deletions Lib/test/test_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -2032,11 +2032,12 @@ def test_user(self):
"import os; print(os.getuid())"],
user=user,
close_fds=close_fds)
except PermissionError: # (EACCES, EPERM)
pass
except PermissionError as e: # (EACCES, EPERM)
self.assertIsNone(e.filename)
except OSError as e:
if e.errno not in (errno.EACCES, errno.EPERM):
raise
self.assertIsNone(e.filename)
else:
if isinstance(user, str):
user_uid = pwd.getpwnam(user).pw_uid
Expand Down Expand Up @@ -2080,8 +2081,8 @@ def test_group(self):
"import os; print(os.getgid())"],
group=group,
close_fds=close_fds)
except PermissionError: # (EACCES, EPERM)
pass
except PermissionError as e: # (EACCES, EPERM)
self.assertIsNone(e.filename)
else:
if isinstance(group, str):
group_gid = grp.getgrnam(group).gr_gid
Expand Down Expand Up @@ -2129,7 +2130,8 @@ def _test_extra_groups_impl(self, *, gid, group_list):
[sys.executable, "-c",
"import os, sys, json; json.dump(os.getgroups(), sys.stdout)"],
extra_groups=group_list)
except PermissionError:
except PermissionError as e:
self.assertIsNone(e.filename)
self.skipTest("setgroup() EPERM; this test may require root.")
else:
parent_groups = os.getgroups()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
:exc:`OSError` raised when run a subprocess now only has *filename*
attribute set to *cwd* if the error was caused by a failed attempt to change
the current directory.
21 changes: 11 additions & 10 deletions Modules/_posixsubprocess.c
Original file line number Diff line number Diff line change
Expand Up @@ -589,9 +589,10 @@ child_exec(char *const exec_array[],
PyObject *preexec_fn,
PyObject *preexec_fn_args_tuple)
{
int i, saved_errno, reached_preexec = 0;
int i, saved_errno;
PyObject *result;
const char* err_msg = "";
/* Indicate to the parent that the error happened before exec(). */
const char *err_msg = "noexec";
/* Buffer large enough to hold a hex integer. We can't malloc. */
char hex_errno[sizeof(saved_errno)*2+1];

Expand Down Expand Up @@ -651,8 +652,12 @@ child_exec(char *const exec_array[],
/* We no longer manually close p2cread, c2pwrite, and errwrite here as
* _close_open_fds takes care when it is not already non-inheritable. */

if (cwd)
POSIX_CALL(chdir(cwd));
if (cwd) {
if (chdir(cwd) == -1) {
err_msg = "noexec:chdir";
goto error;
}
}

if (child_umask >= 0)
umask(child_umask); /* umask() always succeeds. */
Expand Down Expand Up @@ -699,7 +704,7 @@ child_exec(char *const exec_array[],
#endif /* HAVE_SETREUID */


reached_preexec = 1;
err_msg = "";
if (preexec_fn != Py_None && preexec_fn_args_tuple) {
/* This is where the user has asked us to deadlock their program. */
result = PyObject_Call(preexec_fn, preexec_fn_args_tuple, NULL);
Expand Down Expand Up @@ -757,16 +762,12 @@ child_exec(char *const exec_array[],
}
_Py_write_noraise(errpipe_write, cur, hex_errno + sizeof(hex_errno) - cur);
_Py_write_noraise(errpipe_write, ":", 1);
if (!reached_preexec) {
/* Indicate to the parent that the error happened before exec(). */
_Py_write_noraise(errpipe_write, "noexec", 6);
}
/* We can't call strerror(saved_errno). It is not async signal safe.
* The parent process will look the error message up. */
} else {
_Py_write_noraise(errpipe_write, "SubprocessError:0:", 18);
_Py_write_noraise(errpipe_write, err_msg, strlen(err_msg));
}
_Py_write_noraise(errpipe_write, err_msg, strlen(err_msg));
}


Expand Down