Skip to content

Commit f8fc853

Browse files
miss-islingtonserhiy-storchakaPurityLake
authored
[3.12] gh-104522: Fix OSError raised when run a subprocess (GH-114195) (#114219)
gh-104522: Fix OSError raised when run a subprocess (GH-114195) Only set filename to cwd if it was caused by failed chdir(cwd). _fork_exec() now returns "noexec:chdir" for failed chdir(cwd). (cherry picked from commit e2c097e) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> Co-authored-by: Robert O'Shea <PurityLake@users.noreply.github.com>
1 parent 2c9cf64 commit f8fc853

File tree

4 files changed

+29
-18
lines changed

4 files changed

+29
-18
lines changed

‎Lib/subprocess.py‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1938,16 +1938,21 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
19381938
SubprocessError)
19391939
if issubclass(child_exception_type, OSError) and hex_errno:
19401940
errno_num = int(hex_errno, 16)
1941-
child_exec_never_called = (err_msg == "noexec")
1942-
if child_exec_never_called:
1941+
if err_msg == "noexec:chdir":
19431942
err_msg = ""
19441943
# The error must be from chdir(cwd).
19451944
err_filename = cwd
1945+
elif err_msg == "noexec":
1946+
err_msg = ""
1947+
err_filename = None
19461948
else:
19471949
err_filename = orig_executable
19481950
if errno_num != 0:
19491951
err_msg = os.strerror(errno_num)
1950-
raise child_exception_type(errno_num, err_msg, err_filename)
1952+
if err_filename is not None:
1953+
raise child_exception_type(errno_num, err_msg, err_filename)
1954+
else:
1955+
raise child_exception_type(errno_num, err_msg)
19511956
raise child_exception_type(err_msg)
19521957

19531958

‎Lib/test/test_subprocess.py‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,11 +2032,12 @@ def test_user(self):
20322032
"import os; print(os.getuid())"],
20332033
user=user,
20342034
close_fds=close_fds)
2035-
except PermissionError: # (EACCES, EPERM)
2036-
pass
2035+
except PermissionError as e: # (EACCES, EPERM)
2036+
self.assertIsNone(e.filename)
20372037
except OSError as e:
20382038
if e.errno not in (errno.EACCES, errno.EPERM):
20392039
raise
2040+
self.assertIsNone(e.filename)
20402041
else:
20412042
if isinstance(user, str):
20422043
user_uid = pwd.getpwnam(user).pw_uid
@@ -2080,8 +2081,8 @@ def test_group(self):
20802081
"import os; print(os.getgid())"],
20812082
group=group,
20822083
close_fds=close_fds)
2083-
except PermissionError: # (EACCES, EPERM)
2084-
pass
2084+
except PermissionError as e: # (EACCES, EPERM)
2085+
self.assertIsNone(e.filename)
20852086
else:
20862087
if isinstance(group, str):
20872088
group_gid = grp.getgrnam(group).gr_gid
@@ -2129,7 +2130,8 @@ def _test_extra_groups_impl(self, *, gid, group_list):
21292130
[sys.executable, "-c",
21302131
"import os, sys, json; json.dump(os.getgroups(), sys.stdout)"],
21312132
extra_groups=group_list)
2132-
except PermissionError:
2133+
except PermissionError as e:
2134+
self.assertIsNone(e.filename)
21332135
self.skipTest("setgroup() EPERM; this test may require root.")
21342136
else:
21352137
parent_groups = os.getgroups()

‎Misc/NEWS.d/next/Library/2024-01-17-18-53-51.gh-issue-104522.3NyDf4.rst‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:exc:`OSError` raised when run a subprocess now only has *filename*
2+
attribute set to *cwd* if the error was caused by a failed attempt to change
3+
the current directory.

‎Modules/_posixsubprocess.c‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -589,9 +589,10 @@ child_exec(char *const exec_array[],
589589
PyObject *preexec_fn,
590590
PyObject *preexec_fn_args_tuple)
591591
{
592-
int i, saved_errno, reached_preexec = 0;
592+
int i, saved_errno;
593593
PyObject *result;
594-
const char* err_msg = "";
594+
/* Indicate to the parent that the error happened before exec(). */
595+
const char *err_msg = "noexec";
595596
/* Buffer large enough to hold a hex integer. We can't malloc. */
596597
char hex_errno[sizeof(saved_errno)*2+1];
597598

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

654-
if (cwd)
655-
POSIX_CALL(chdir(cwd));
655+
if (cwd) {
656+
if (chdir(cwd) == -1) {
657+
err_msg = "noexec:chdir";
658+
goto error;
659+
}
660+
}
656661

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

701706

702-
reached_preexec = 1;
707+
err_msg = "";
703708
if (preexec_fn != Py_None && preexec_fn_args_tuple) {
704709
/* This is where the user has asked us to deadlock their program. */
705710
result = PyObject_Call(preexec_fn, preexec_fn_args_tuple, NULL);
@@ -757,16 +762,12 @@ child_exec(char *const exec_array[],
757762
}
758763
_Py_write_noraise(errpipe_write, cur, hex_errno + sizeof(hex_errno) - cur);
759764
_Py_write_noraise(errpipe_write, ":", 1);
760-
if (!reached_preexec) {
761-
/* Indicate to the parent that the error happened before exec(). */
762-
_Py_write_noraise(errpipe_write, "noexec", 6);
763-
}
764765
/* We can't call strerror(saved_errno). It is not async signal safe.
765766
* The parent process will look the error message up. */
766767
} else {
767768
_Py_write_noraise(errpipe_write, "SubprocessError:0:", 18);
768-
_Py_write_noraise(errpipe_write, err_msg, strlen(err_msg));
769769
}
770+
_Py_write_noraise(errpipe_write, err_msg, strlen(err_msg));
770771
}
771772

772773

0 commit comments

Comments
 (0)