Skip to content

Commit

Permalink
No need to loop reading from/writing to a blocking socketpair.
Browse files Browse the repository at this point in the history
This removes some infinite loops that can cause static analyzer
warnings.  The fds are not in non-blocking mode and we use restartable
system calls so there is no need to loop.
  • Loading branch information
millert committed Sep 22, 2023
1 parent 1c7d757 commit 171abbe
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 17 deletions.
24 changes: 11 additions & 13 deletions src/exec_monitor.c
Original file line number Diff line number Diff line change
Expand Up @@ -378,12 +378,10 @@ exec_cmnd_pty(struct command_details *details, sigset_t *mask,

sudo_debug_printf(SUDO_DEBUG_INFO, "%s: waiting for controlling tty",
__func__);
while (recv(errfd, &ch, sizeof(ch), 0) == -1) {
if (errno != EINTR) {
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
"%s: unable to receive message from parent", __func__);
debug_return;
}
if (recv(errfd, &ch, sizeof(ch), 0) == -1) {
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
"%s: unable to receive message from parent", __func__);
debug_return;
}
if (tcgetpgrp(io_fds[SFD_FOLLOWER]) == self) {
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: got controlling tty",
Expand Down Expand Up @@ -595,11 +593,9 @@ exec_monitor(struct command_details *details, sigset_t *oset,
* Before forking, wait for the main sudo process to tell us to go.
* Avoids race conditions when the command exits quickly.
*/
while (recv(backchannel, &cstat, sizeof(cstat), MSG_WAITALL) == -1) {
if (errno != EINTR && errno != EAGAIN) {
sudo_warn("%s", U_("unable to receive message from parent"));
goto bad;
}
if (recv(backchannel, &cstat, sizeof(cstat), MSG_WAITALL) == -1) {
sudo_warn("%s", U_("unable to receive message from parent"));
goto bad;
}

#ifdef HAVE_SELINUX
Expand Down Expand Up @@ -676,8 +672,10 @@ exec_monitor(struct command_details *details, sigset_t *oset,
__func__, (int)mc.cmnd_pgrp);
}
/* Tell the child to go ahead now that it is the foreground pgrp. */
while (send(errsock[0], "", 1, 0) == -1 && errno == EINTR)
continue;
if (send(errsock[0], "", 1, 0) == -1) {
sudo_warn(U_("unable to execute %s"), details->command);
terminate_command(mc.cmnd_pid, true);
}
}

/*
Expand Down
6 changes: 2 additions & 4 deletions src/exec_pty.c
Original file line number Diff line number Diff line change
Expand Up @@ -1322,10 +1322,8 @@ exec_pty(struct command_details *details,
/* Tell the monitor to continue now that the follower is closed. */
cstat->type = CMD_SIGNO;
cstat->val = 0;
while (send(sv[0], cstat, sizeof(*cstat), 0) == -1) {
if (errno != EINTR && errno != EAGAIN)
sudo_fatal("%s", U_("unable to send message to monitor process"));
}
if (send(sv[0], cstat, sizeof(*cstat), 0) == -1)
sudo_fatal("%s", U_("unable to send message to monitor process"));

/* Close the other end of the stdin/stdout/stderr pipes and socketpair. */
if (io_pipe[STDIN_FILENO][0] != -1)
Expand Down

0 comments on commit 171abbe

Please sign in to comment.