gspawn: Check from errors from safe_dup2() and dupfd_cloexec()

Although unlikely, these functions can fail, e.g. if we run out of file
descriptors. Check for errors to improve robustness. This is especially
important now that I changed our use of dupfd_cloexec() to avoid
returning fds smaller than the largest fd in target_fds. An application
that attempts to remap to the highest-allowed fd value deserves at least
some sort of attempt at error reporting, not silent failure.
This commit is contained in:
Michael Catanzaro 2021-12-14 13:36:30 -06:00
parent 2a7547f8a5
commit 34de33a9bd

View File

@ -1660,7 +1660,6 @@ do_exec (gint child_err_report_fd,
/* Redirect pipes as required */ /* Redirect pipes as required */
if (stdin_fd >= 0) if (stdin_fd >= 0)
{ {
/* dup2 can't actually fail here I don't think */
if (safe_dup2 (stdin_fd, 0) < 0) if (safe_dup2 (stdin_fd, 0) < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED); CHILD_DUP2_FAILED);
@ -1676,13 +1675,14 @@ do_exec (gint child_err_report_fd,
if (read_null < 0) if (read_null < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED); CHILD_DUP2_FAILED);
safe_dup2 (read_null, 0); if (safe_dup2 (read_null, 0) < 0)
write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED);
close_and_invalidate (&read_null); close_and_invalidate (&read_null);
} }
if (stdout_fd >= 0) if (stdout_fd >= 0)
{ {
/* dup2 can't actually fail here I don't think */
if (safe_dup2 (stdout_fd, 1) < 0) if (safe_dup2 (stdout_fd, 1) < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED); CHILD_DUP2_FAILED);
@ -1697,13 +1697,14 @@ do_exec (gint child_err_report_fd,
if (write_null < 0) if (write_null < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED); CHILD_DUP2_FAILED);
safe_dup2 (write_null, 1); if (safe_dup2 (write_null, 1) < 0)
write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED);
close_and_invalidate (&write_null); close_and_invalidate (&write_null);
} }
if (stderr_fd >= 0) if (stderr_fd >= 0)
{ {
/* dup2 can't actually fail here I don't think */
if (safe_dup2 (stderr_fd, 2) < 0) if (safe_dup2 (stderr_fd, 2) < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED); CHILD_DUP2_FAILED);
@ -1718,7 +1719,9 @@ do_exec (gint child_err_report_fd,
if (write_null < 0) if (write_null < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED); CHILD_DUP2_FAILED);
safe_dup2 (write_null, 2); if (safe_dup2 (write_null, 2) < 0)
write_err_and_exit (child_err_report_fd,
CHILD_DUP2_FAILED);
close_and_invalidate (&write_null); close_and_invalidate (&write_null);
} }
@ -1731,7 +1734,8 @@ do_exec (gint child_err_report_fd,
{ {
if (child_setup == NULL && n_fds == 0) if (child_setup == NULL && n_fds == 0)
{ {
safe_dup2 (child_err_report_fd, 3); if (safe_dup2 (child_err_report_fd, 3) < 0)
write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED);
set_cloexec (GINT_TO_POINTER (0), 3); set_cloexec (GINT_TO_POINTER (0), 3);
safe_closefrom (4); safe_closefrom (4);
child_err_report_fd = 3; child_err_report_fd = 3;
@ -1776,7 +1780,11 @@ do_exec (gint child_err_report_fd,
for (i = 0; i < n_fds; i++) for (i = 0; i < n_fds; i++)
{ {
if (source_fds[i] != target_fds[i]) if (source_fds[i] != target_fds[i])
source_fds[i] = dupfd_cloexec (source_fds[i], max_target_fd + 1); {
source_fds[i] = dupfd_cloexec (source_fds[i], max_target_fd + 1);
if (source_fds[i] < 0)
write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED);
}
} }
for (i = 0; i < n_fds; i++) for (i = 0; i < n_fds; i++)
@ -1794,9 +1802,15 @@ do_exec (gint child_err_report_fd,
* dup it so it doesnt get conflated. * dup it so it doesnt get conflated.
*/ */
if (target_fds[i] == child_err_report_fd) if (target_fds[i] == child_err_report_fd)
child_err_report_fd = dupfd_cloexec (child_err_report_fd, max_target_fd + 1); {
child_err_report_fd = dupfd_cloexec (child_err_report_fd, max_target_fd + 1);
if (child_err_report_fd < 0)
write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED);
}
if (safe_dup2 (source_fds[i], target_fds[i]) < 0)
write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED);
safe_dup2 (source_fds[i], target_fds[i]);
close_and_invalidate (&source_fds[i]); close_and_invalidate (&source_fds[i]);
} }
} }
@ -2042,7 +2056,11 @@ do_posix_spawn (const gchar * const *argv,
duped_source_fds = g_new (gint, n_fds); duped_source_fds = g_new (gint, n_fds);
for (i = 0; i < n_fds; i++) for (i = 0; i < n_fds; i++)
duped_source_fds[i] = dupfd_cloexec (source_fds[i], max_target_fd + 1); {
duped_source_fds[i] = dupfd_cloexec (source_fds[i], max_target_fd + 1);
if (duped_source_fds[i] < 0)
goto out_close_fds;
}
for (i = 0; i < n_fds; i++) for (i = 0; i < n_fds; i++)
{ {