Bug#253099: killall.sh: Can kill wrong PID

[Thomas Hood <jdthood@yahoo.co.uk>]

Package: netcfg
Version: 0.67
Severity: minor
Tags: patch

The killall.sh script uses the following code to find the PIDs of running
DHCP client daemons:

pids=$(ps ax | grep 'udhcpc\|dhclient\|pump' | sed 's/^[
]*\([0-9]*\).*/\1/')

However, (1) this picks up the PID of the grep process itself:

$ ps ax | grep 'udhcpc\|dhclient\|pump'
15769 pts/2    S+     0:00 grep udhcpc\|dhclient\|pump


(2) I don't see where udhcpc is started.  In dhcp.c only
dhclient3, dhclient and pump seem to be supported.

(3) ps has built in output formatting so sed isn't really needed.

Better would be:

for CLIENT in dhclient3 dhclient pump ; do
  pids=$(ps --no-headers -o pid -C $CLIENT)
  for pid in $pids; do
  ...
  done
done


Here's a patch:

--- /tmp/killall.sh_ORIG	2004-06-07 10:24:26.000000000 +0200
+++ killall.sh	2004-06-07 10:26:34.000000000 +0200
@@ -1,15 +1,15 @@
 #!/bin/sh
 # Killall for dhcp clients.
 
-pids=$(ps ax | grep 'udhcpc\|dhclient\|pump' | sed 's/^[ ]*\([0-9]*\).*/\1/')
-
-for pid in $pids; do
-  if kill -0 $pid 2>/dev/null; then
-    kill -TERM $pid
-    sleep 1
-    # Still alive? Die!
+for CLIENT in dhclient3 dhclient pump ; do
+  for pid in $(ps --no-headers -o pid -C $CLIENT) ; do
     if kill -0 $pid 2>/dev/null; then
-      kill -KILL $pid
+      kill -TERM $pid
+      sleep 1
+      # Still alive? Die!
+      if kill -0 $pid 2>/dev/null; then
+        kill -KILL $pid
+      fi
     fi
-  fi
+  done
 done


Another thing: One second isn't very long.  What happens if this code
kill -KILL's the DHCP client before it has finished deconfiguring the
interface?  Perhaps start-stop-daemon should be used here with a
longer timeout.

for CLIENT in /sbin/dhclient3 /sbin/dhclient /sbin/pump ; do
  [ -x $CLIENT ] && start-stop-daemon --stop --quiet --oknodo --exec $CLIENT --retry=TERM/10/KILL/2 > /dev/null
done

This has the additional advantage of only killing genuine processes,
not processes whose executable files happen to have the same names.


-- System Information:
Debian Release: testing/unstable
  APT prefers unstable
  APT policy: (900, 'unstable'), (700, 'testing'), (1, 'experimental')
Architecture: i386 (i686)
Kernel: Linux 2.4.25
Locale: LANG=en_IE@euro, LC_CTYPE=en_IE@euro