From 7eca1112fbd7a3eae6a6cbc6e52482a0a8326ff7 Mon Sep 17 00:00:00 2001
From: "Todd C. Miller" <Todd.Miller@sudo.ws>
Date: Mon, 11 Nov 2019 20:08:10 -0600
Subject: [PATCH] Fix vipw not resuming correctly when suspended

Closes #185

If vipw is suspended (e.g. via control-Z) and then resumed, it often gets
immediately suspended. This is easier to reproduce on a multi-core system.

root@buster:~# /usr/sbin/vipw

[1]+  Stopped                 /usr/sbin/vipw
root@buster:~# fg
/usr/sbin/vipw

[1]+  Stopped                 /usr/sbin/vipw

root@buster:~# fg
[vipw resumes on the second fg]

The problem is that vipw forks a child process and calls waitpid() with the
WUNTRACED flag. When the child process (running the editor) is suspended, the
parent sends itself SIGSTOP to suspend the main vipw process. However, because
the main vipw is in the same process group as the editor which received the ^Z,
the kernel already sent the main vipw SIGTSTP.

If the main vipw receives SIGTSTP before the child, it will be suspended and
then, once resumed, will proceed to suspend itself again.

To fix this, run the child process in its own process group as the foreground
process group. That way, control-Z will only affect the child process and the
parent can use the existing logic to suspend the parent.
---
 src/vipw.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/src/vipw.c b/src/vipw.c
index 2cfac6b4..04f0b233 100644
--- a/src/vipw.c
+++ b/src/vipw.c
@@ -207,6 +207,8 @@ vipwedit (const char *file, int (*file_lock) (void), int (*file_unlock) (void))
 	struct stat st1, st2;
 	int status;
 	FILE *f;
+	pid_t orig_pgrp, editor_pgrp = -1;
+	sigset_t mask, omask;
 	/* FIXME: the following should have variable sizes */
 	char filebackup[1024], fileedit[1024];
 	char *to_rename;
@@ -294,6 +296,8 @@ vipwedit (const char *file, int (*file_lock) (void), int (*file_unlock) (void))
 		editor = DEFAULT_EDITOR;
 	}
 
+	orig_pgrp = tcgetpgrp(STDIN_FILENO);
+
 	pid = fork ();
 	if (-1 == pid) {
 		vipwexit ("fork", 1, 1);
@@ -303,6 +307,14 @@ vipwedit (const char *file, int (*file_lock) (void), int (*file_unlock) (void))
 		char *buf;
 		int status;
 
+		/* Wait for parent to make us the foreground pgrp. */
+		if (orig_pgrp != -1) {
+			pid = getpid();
+			setpgid(0, 0);
+			while (tcgetpgrp(STDIN_FILENO) != pid)
+				continue;
+		}
+
 		buf = (char *) malloc (strlen (editor) + strlen (fileedit) + 2);
 		snprintf (buf, strlen (editor) + strlen (fileedit) + 2,
 		          "%s %s", editor, fileedit);
@@ -325,19 +337,50 @@ vipwedit (const char *file, int (*file_lock) (void), int (*file_unlock) (void))
 		}
 	}
 
+	/* Run child in a new pgrp and make it the foreground pgrp. */
+	if (orig_pgrp != -1) {
+		setpgid(pid, pid);
+		tcsetpgrp(STDIN_FILENO, pid);
+
+		/* Avoid SIGTTOU when changing foreground pgrp below. */
+		sigemptyset(&mask);
+		sigaddset(&mask, SIGTTOU);
+		sigprocmask(SIG_BLOCK, &mask, &omask);
+	}
+
 	for (;;) {
 		pid = waitpid (pid, &status, WUNTRACED);
 		if ((pid != -1) && (WIFSTOPPED (status) != 0)) {
 			/* The child (editor) was suspended.
-			 * Suspend vipw. */
+			 * Restore terminal pgrp and suspend vipw. */
+			if (orig_pgrp != -1) {
+				editor_pgrp = tcgetpgrp(STDIN_FILENO);
+				if (editor_pgrp == -1) {
+					fprintf (stderr, "%s: %s: %s", Prog,
+						 "tcgetpgrp", strerror (errno));
+				}
+				if (tcsetpgrp(STDIN_FILENO, orig_pgrp) == -1) {
+					fprintf (stderr, "%s: %s: %s", Prog,
+						 "tcsetpgrp", strerror (errno));
+				}
+			}
 			kill (getpid (), SIGSTOP);
 			/* wake child when resumed */
-			kill (pid, SIGCONT);
+			if (editor_pgrp != -1) {
+				if (tcsetpgrp(STDIN_FILENO, editor_pgrp) == -1) {
+					fprintf (stderr, "%s: %s: %s", Prog,
+						 "tcsetpgrp", strerror (errno));
+				}
+			}
+			killpg (pid, SIGCONT);
 		} else {
 			break;
 		}
 	}
 
+	if (orig_pgrp != -1)
+		sigprocmask(SIG_SETMASK, &omask, NULL);
+
 	if (-1 == pid) {
 		vipwexit (editor, 1, 1);
 	} else if (   WIFEXITED (status)