Your message dated Sun, 14 Nov 2021 10:44:12 +0100 with message-id <CAK0Odpw0Qnm4OFD-xdNXP8TpF=w4ybnwpxq0aqttjr8pz7c...@mail.gmail.com> and subject line Re: Bug#942680: [Pkg-shadow-devel] Bug#942680: passwd: vipw does not resume properly when suspended has caused the Debian Bug report #942680, regarding passwd: vipw does not resume properly when suspended to be marked as done.
This means that you claim that the problem has been dealt with. If this is not the case it is now your responsibility to reopen the Bug report if necessary, and/or fix the problem forthwith. (NB: If you are a system administrator and have no idea what this message is talking about, this may indicate a serious mail system misconfiguration somewhere. Please contact [email protected] immediately.) -- 942680: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=942680 Debian Bug Tracking System Contact [email protected] with problems
--- Begin Message ---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
--- End Message ---
--- Begin Message ---Control: fixed -1 4.8-1 Control: forwarded -1 https://github.com/shadow-maint/shadow/issues/185 Control: upstream Todd C. Miller <[email protected]> ezt írta (időpont: 2019. nov. 4., H, 19:15): > > On Sat, 26 Oct 2019 07:49:33 -0500, "Serge E. Hallyn" wrote: > > > second option sounds nicer but sure is a lot more code. So I'm > > leaning towards the first. Do you mind creating a github issue > > at github.com/shadow-maint/shadow for this, or would you prefer that > > I do it? > > I opened a github issue and attached the patches: > https://github.com/shadow-maint/shadow/issues/185 > > - todd >
--- End Message ---
_______________________________________________ Pkg-shadow-devel mailing list [email protected] https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/pkg-shadow-devel
