Package: passwd Version: 1:4.5-1.1 Severity: normal Tags: patch Dear Maintainer,
* What led up to the situation? 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. * What exactly did you do (or not do) that was effective (or ineffective)? 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. There are two main ways to fix this. I've included patches for each approach. 1) Don't pass the WUNTRACED flag to waitpid() in vipw.c and just assume the main vipw will receive a stop signal from the kernel. This is the simplest fix and works fine for stop signals generated due to ^Z. However, if someone sends SIGSTOP to the vipw child process via the kill command, the parent vipw will not notice. --- vipw.c.orig 2019-10-18 16:19:15.673956247 -0600 +++ vipw.c.simple_fix 2019-10-18 16:43:04.265583507 -0600 @@ -325,16 +325,9 @@ } for (;;) { - pid = waitpid (pid, &status, WUNTRACED); - if ((pid != -1) && (WIFSTOPPED (status) != 0)) { - /* The child (editor) was suspended. - * Suspend vipw. */ - kill (getpid (), SIGSTOP); - /* wake child when resumed */ - kill (pid, SIGCONT); - } else { + pid = waitpid (pid, &status, 0); + if (pid != -1 || errno != EINTR) break; - } } if (-1 == pid) { 2) The other option is to run the child process in its own process group as the foregroup process group. That way, control-Z will only affect the child process and the parent can use the existing logic to suspend the parent. --- vipw.c.orig 2019-10-18 16:19:15.673956247 -0600 +++ vipw.c.pgrp_fix 2019-10-19 12:55:50.526591299 -0600 @@ -206,6 +206,8 @@ 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; @@ -293,6 +295,8 @@ editor = DEFAULT_EDITOR; } + orig_pgrp = tcgetpgrp(STDIN_FILENO); + pid = fork (); if (-1 == pid) { vipwexit ("fork", 1, 1); @@ -302,6 +306,14 @@ 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); @@ -324,19 +336,50 @@ } } + /* 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) -- System Information: Debian Release: 10.1 APT prefers stable-updates APT policy: (500, 'stable-updates'), (500, 'stable') Architecture: amd64 (x86_64) Kernel: Linux 4.19.0-6-amd64 (SMP w/2 CPU cores) Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en_US.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /usr/bin/dash Init: systemd (via /run/systemd/system) LSM: AppArmor: enabled Versions of packages passwd depends on: ii libaudit1 1:2.8.4-3 ii libc6 2.28-10 ii libpam-modules 1.3.1-5 ii libpam0g 1.3.1-5 ii libselinux1 2.8-1+b1 ii libsemanage1 2.8-2 passwd recommends no packages. passwd suggests no packages. -- no debconf information