emo/shadow
1
0
Fork 0
Code Issues Pull Requests Packages Releases Activity

Fix vipw not resuming correctly when suspended

Browse Source 
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.
This commit is contained in:
Todd C. Miller 2019-11-11 20:08:10 -06:00 committed by Serge Hallyn
parent fe2a266c50
commit 7eca1112fb
1 changed files with 45 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
src/vipw.c
View File

@ -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)