Bug#968038: sysvinit-utils: do_restart fails when not ran from a terminal

[Stephane Lapie]

Package: sysvinit-utils
Version: 2.93-8
Severity: important
Tags: patch

Dear Maintainer,

   * What led up to the situation?

I am running Debian Buster (with sysvinit), and tried to backport net-snmp,
stumbling upon the same issue as #932978.

This was when I realized the trigger was upgrading the package snmpd,
because its postinst script failed.

   * What exactly did you do (or not do) that was effective (or
     ineffective)?

Upgrading the snmpd package runs a 'restart' command within the context of dpkg.
In turn, /etc/init.d/snmpd gets called, sourcing /lib/init/init-d-script,
and the do_restart function is called.

I fixed the immediate issue on my side with the diff mentioned here :
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=932978#35

But to properly fix the problem,
functions do_start() and do_stop() would likely need the same kind of fix :
- Storing the actual result of the command in a variable,
using that variable to decide whether to call log_end_msg
- If there is a cleanup hook, store its result and return it
- Otherwise, return the result

Here is my proposed patch for the whole thing:
--- /lib/init/init-d-script.old	2020-08-07 17:41:22.256239171 +0900
+++ /lib/init/init-d-script	2020-08-07 17:41:22.492236691 +0900
@@ -68,13 +68,16 @@
 	fi
 	[ "$VERBOSE" != no ] && log_daemon_msg "Starting $DESC" "$NAME"
 	call do_start_cmd
-	case "$?" in
+	result=$?
+	case "$result" in
 		0|1) [ "$VERBOSE" != no ] && log_end_msg 0 ;;
 		2) [ "$VERBOSE" != no ] && log_end_msg 1 ;;
 	esac
 	if is_call_implemented do_start_cleanup ; then
 		call do_start_cleanup
+		result=$?
 	fi
+	return $result
 }
 
 #
@@ -114,23 +117,28 @@
 	fi
 	[ "$VERBOSE" != no ] && log_daemon_msg "Stopping $DESC" "$NAME"
 	call do_stop_cmd
-	case "$?" in
+	result=$?
+	case "$result" in
 		0|1) [ "$VERBOSE" != no ] && log_end_msg 0 ;;
 		2) [ "$VERBOSE" != no ] && log_end_msg 1 ;;
 	esac
 	if is_call_implemented do_stop_cleanup ; then
 		call do_stop_cleanup
+		result=$?
 	fi
+	return $result
 }
 
 do_restart() {
 	[ "$VERBOSE" != no ] && log_daemon_msg "Restarting $DESC" "$NAME"
 	call do_stop_cmd
 	call do_start_cmd
-	case "$?" in
+	result=$?
+	case "$result" in
 		0|1) [ "$VERBOSE" != no ] && log_end_msg 0 ;;
 		2) [ "$VERBOSE" != no ] && log_end_msg 1 ;;
 	esac
+	return $result
 }
 
 do_force_reload() {


   * What was the outcome of this action?

Without my patch,
the do_restart function runs do_stop, do_start successfully,
but then it checks for verbosity to know whether it must display a message or not,
and the boolean result of that verbosity check ends up being returned to the rest of the script.

The result is that :
- if ran from a TTY, VERBOSE ends up set as 'yes', a message is displayed and it succeeds.
- if not ran from a TTY, VERBOSE is set as 'no', the check fails, and the whole script exits
with this return value.

   * What outcome did you expect instead?

I would expect the verbosity check value to not impact the success of the script,
and behavior to be functionally identical whether there is a TTY or not.

My patch ensures that the verbosity check result does not contaminate the returned result.

-- System Information:
Debian Release: 10.5
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 4.19.0-9-amd64 (SMP w/2 CPU cores)
Locale: LANG=C.UTF-8, LC_CTYPE=C.UTF-8 (charmap=UTF-8), LANGUAGE=C.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /usr/bin/dash
Init: sysvinit (via /sbin/init)
LSM: AppArmor: enabled

Versions of packages sysvinit-utils depends on:
ii  init-system-helpers  1.56+nmu1
ii  libc6                2.28-10
ii  util-linux           2.33.1-0.1

sysvinit-utils recommends no packages.

sysvinit-utils suggests no packages.

-- no debconf information