[pkg-dhcp-devel] Bug#692846: isc-dhcp-client: /sbin/dhclient-script incorrectly handles exit codes

[Philipp Hahn]

Package: isc-dhcp-client
Version: 4.2.4-3
Severity: important

/sbin/dhclient-script contains several logic bugs, which break proper handling of return codes.

Reading "man dhcp-scipts":
> HOOKS
>   ... It also invokes all executable scripts in ETCDIR/dhclient-enter-hooks.d/* in the same way.
>   ... If an error occurs during the execution of the script, it can set the exit_status variable to a nonzero value, and CLIENTBINDIR/dhclient-script will exit with that error code  immediately after the client script exits.

/sbin/dhclient-script:128
> run_hookdir() {
>     local dir
>     local exit_status
>     dir="$1"
>     shift	# See run_hook
> 
>     if [ -d "$dir" ]; then
>         for script in $(run-parts --list $dir); do
>             run_hook $script "$@" || true
By appending the "|| true" the exit status in now alyways 0
>             exit_status=$?
>         done
>     fi

/sbin/dhclient-script:149
>    if ! run_hook /etc/dhcp/dhclient-exit-hooks "$@"; then
>        exit_status=$?
exit_status will be alwys 0, since $? now contains the value of the "! runhook" status.


Please have a look at the upstream isc-dhcp-4.1.1-P1/client/scripts/linux
file or take a look at the following patch, which (hopefully) gets it right:
--- /sbin/dhclient-script	2012-10-15 21:03:17.000000000 +0200
+++ /home/phahn/dhclient-scipt	2012-11-08 23:22:45.449909285 +0100
@@ -109,36 +109,31 @@
 # run given script
 run_hook() {
     local script
-    local exit_status
+    local prev_exit_status
     script="$1"
+    prev_exit_status="$exit_status"
     shift	# discard the first argument, then the rest are the script's
 
     if [ -f $script ]; then
         . $script "$@"
     fi
 
-    if [ -n "$exit_status" ] && [ "$exit_status" -ne 0 ]; then
-        logger -p daemon.err "$script returned non-zero exit status $exit_status"
+    if [ "$exit_status" -ne "$prev_exit_status" ]; then
+        logger -p daemon.err "$script returned exit status $exit_status"
     fi
-
-    return $exit_status
 }
 
 # run scripts in given directory
 run_hookdir() {
     local dir
-    local exit_status
     dir="$1"
     shift	# See run_hook
 
     if [ -d "$dir" ]; then
         for script in $(run-parts --list $dir); do
-            run_hook $script "$@" || true
-            exit_status=$?
+            run_hook $script "$@"
         done
     fi
-
-    return $exit_status
 }
 
 # Must be used on exit.   Invokes the local dhcp client exit hooks, if any.
@@ -146,14 +141,10 @@
     exit_status=$1
 
     # Source the documented exit-hook script, if it exists
-    if ! run_hook /etc/dhcp/dhclient-exit-hooks "$@"; then
-        exit_status=$?
-    fi
+    run_hook /etc/dhcp/dhclient-exit-hooks "$@"
 
     # Now run scripts in the Debian-specific directory.
-    if ! run_hookdir /etc/dhcp/dhclient-exit-hooks.d "$@"; then
-        exit_status=$?
-    fi
+    run_hookdir /etc/dhcp/dhclient-exit-hooks.d "$@"
 
     exit $exit_status
 }
@@ -172,8 +163,12 @@
 # The action starts here
 
 # Invoke the local dhcp client enter hooks, if they exist.
+exit_status=0
 run_hook /etc/dhcp/dhclient-enter-hooks
 run_hookdir /etc/dhcp/dhclient-enter-hooks.d
+if [ $exit_status -ne 0 ]; then
+    exit $exit_status
+fi
 
 # Execute the operation
 case "$reason" in

-- System Information:
Debian Release: wheezy/sid
  APT prefers unstable
  APT policy: (500, 'unstable')
Architecture: amd64 (x86_64)

Kernel: Linux 3.2.0-4-amd64 (SMP w/1 CPU core)
Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages isc-dhcp-client depends on:
ii  debianutils      4.3.4
ii  iproute          20120521-3
ii  isc-dhcp-common  4.2.4-3
ii  libc6            2.13-36

isc-dhcp-client recommends no packages.

Versions of packages isc-dhcp-client suggests:
pn  avahi-autoipd  <none>
pn  resolvconf     <none>

-- no debconf information