In a couple places, use set -m to cause subshells to gain process

groups, and then kill the process group instead of the ksh pid. Some
of these processes contain sleep, which kept running, and in some
cases retained stderr (or other fd) and confused parent processes.
In some cases, add manual wait.  Finally, store the pid (nee pgrp)
in /tmp/xxpid files rather than variables, since there is a bit
of recursion and sub-shell confusion happening, and we have confused
ourselves at least twice with these pid variables not being in scope.
ok beck, with florian, ok kn
In snaps for almost a week.  A few more tweaks may come in a while.

diff --git a/distrib/miniroot/dot.profile b/distrib/miniroot/dot.profile
index c630cd2..8a47b33 100644 (file)
--- a/distrib/miniroot/dot.profile
+++ b/distrib/miniroot/dot.profile
@@ -1,4 +1,4 @@
-#      $OpenBSD: dot.profile,v 1.49 2021/09/08 13:16:53 kn Exp $
+#      $OpenBSD: dot.profile,v 1.50 2022/05/09 22:42:53 deraadt Exp $
 #      $NetBSD: dot.profile,v 1.1 1995/12/18 22:54:43 pk Exp $
 #
 # Copyright (c) 2009 Kenneth R. Westerback
@@ -47,18 +47,23 @@ TIMEOUT_PERIOD_SEC=5
 
 # Stop the background timer.
 stop_timeout() {
-       kill -KILL $WDPID 2>/dev/null
+       local _pid;
+       if [ -f /tmp/dotpid ]; then
+               _pid=$(cat /tmp/dotpid)
+               kill -KILL -$_pid 2>/dev/null
+               wait $_pid 2>/dev/null
+               rm /tmp/dotpid
+       fi
 }
 
-# Start a co-process to XXX.
+# Start a timeout process, in case install gets hung somehow
 start_timeout() {
+       set -m
        (
                sleep $TIMEOUT_PERIOD_SEC && kill $$
-       ) |&
-       WDPID=$!
-
-       # Close standard input of the co-process.
-       exec 3>&p; exec 3>&-
+       ) &
+       echo $! > /tmp/dotpid
+       set +m
 }
 
 if [[ -z $DONEPROFILE ]]; then

diff --git a/distrib/miniroot/install.sub b/distrib/miniroot/install.sub
index e7ad9da..a392a4c 100644 (file)
--- a/distrib/miniroot/install.sub
+++ b/distrib/miniroot/install.sub
@@ -1,5 +1,5 @@
 #!/bin/ksh
-#      $OpenBSD: install.sub,v 1.1196 2022/05/05 20:07:23 florian Exp $
+#      $OpenBSD: install.sub,v 1.1197 2022/05/09 22:42:53 deraadt Exp $
 #
 # Copyright (c) 1997-2015 Todd Miller, Theo de Raadt, Ken Westerback
 # Copyright (c) 2015, Robert Peichaer <rpe@openbsd.org>
@@ -76,7 +76,10 @@ usage() {
 wait_cgiinfo() {
        local _l _s _key _val
 
-       wait "$CGIPID" 2>/dev/null
+       if [ -f /tmp/cgipid ]; then
+               wait "$(cat /tmp/cgipid)" 2>/dev/null
+               rm -f /tmp/cgipid
+       fi
 
        # Ensure, there is actual data to extract info from.
        [[ -s $CGI_INFO ]] || return
@@ -540,7 +543,7 @@ unlock() {
 
 # Add a trap to kill the dmesg listener co-process on exit of the installer.
 retrap() {
-       trap 'kill -KILL $CPPID 2>/dev/null; echo; stty echo; exit 0' \
+       trap 'if [ -f /tmp/cppid ]; then kill -KILL -$(cat /tmp/cppid) 2>/dev/null; rm -f /tmp/cppid; fi; echo; stty echo; exit 0' \
                INT EXIT TERM
 }
 
@@ -559,6 +562,7 @@ start_dmesg_listener() {
 
        # To ensure that only one dmesg listener instance can be active, run it
        # in a co-process subshell of which there can always only be one active.
+       set -m
        (
        while :; do
                lock
@@ -568,17 +572,17 @@ start_dmesg_listener() {
                # contents of that file.
                if [[ -e $_update && "$(dmesgtail)" != "$(<$_update)" ]]; then
                        dmesgtail >$_update
+                       rm -f /tmp/cppid
                        kill -TERM 2>/dev/null $$ || exit 1
                fi
                unlock
                sleep .5
        done
        ) |&
-
-       # Save the co-process PID in a global variable so it can be used in
-       # the retrap() function which adds a trap to kill the co-process on
-       # exit of the installer script.
-       CPPID=$!
+       # Save the co-process pid so it can be used in the retrap() function which
+       # adds a trap to kill the co-process on exit of the installer script.
+       echo $! > /tmp/cppid
+       set +m
        retrap
 }
 
@@ -2551,13 +2555,18 @@ start_cgiinfo() {
                # Remember finish time for adjusting the received timestamp.
                echo -n $SECONDS >$HTTP_SEC
                feed_random
-       ) & CGIPID=$!
+       ) &
+       echo $! > /tmp/cgipid
        set +m
 
        # If the ftp process takes more than 12 seconds, kill it.
-       # XXX We are relying on the pid space not randomly biting us.
-       # XXX ftp could terminate early, and the pid could be reused.
-       (sleep 12; kill -INT -$CGIPID >/dev/null 2>&1) &
+       (
+               sleep 12;
+               if [ -f /tmp/cgipid ]; then
+                       kill -INT -"$(cat /tmp/cgipid)" >/dev/null 2>&1
+                       # wait will be done by wait_cgiinfo
+               fi
+       ) &
 }
 
 # Create a skeletal but useful /etc/fstab from /tmp/i/fstab by stripping all
@@ -3304,7 +3313,11 @@ do_upgrade() {
 
        # Perform final steps common to both an install and an upgrade.
        finish_up
-       [[ -n $WDPID ]] && kill -KILL $WDPID 2>/dev/null
+       if [ -f /tmp/wdpid ]; then
+               kill -KILL "$(cat /tmp/wdpid)" 2>/dev/null
+               # do not bother waiting
+               rm -f /tmp/wdpid
+       fi
 }
 
 check_unattendedupgrade() {
@@ -3331,8 +3344,12 @@ WATCHDOG_PERIOD_SEC=$((30 * 60))
 
 # Restart the background timer.
 reset_watchdog() {
-       if [[ -n $WDPID ]]; then
-               kill -KILL $WDPID 2>/dev/null
+       local _pid
+       if [ -f /tmp/wdpid ]; then
+               _pid=$(cat /tmp/wdpid)
+               kill -KILL -$_pid 2>/dev/null
+               wait $_pid 2>/dev/null
+               rm -f /tmp/wdpid
                start_watchdog
        fi
 }
@@ -3340,10 +3357,12 @@ reset_watchdog() {
 # Start a process to reboot a stalled sysupgrade.
 # This mechanism is only used during non-interactive sysupgrade.
 start_watchdog() {
+       set -m
        (
                sleep $WATCHDOG_PERIOD_SEC && echo WATCHDOG > /dev/tty && reboot
        ) >/dev/null 2>&1 &
-       WDPID=$!
+       echo $! > /tmp/wdpid
+       set +m
 }
 
 # ------------------------------------------------------------------------------
@@ -3496,7 +3515,6 @@ elif $UU; then
        check_unattendedupgrade || exit 1
 
        start_watchdog
-       export WDPID
 
        get_responsefile
        do_autoinstall