2 Fixes for Debian Bug#623425

Both patches are by Jonathan Nieder <jrnieder@gmail.com>

Ever since psmisc 22.4 (2007-04-08), "killall <path>" compares
the target of the /proc/<pid>/exe link to <path> when comparison
of inode numbers fails.  But the code to do this has two problems:

 - readlink does not NUL-terminate, but we use strcmp to
   compare strings.  Probably this hasn't been a problem so far
   because (1) the on-stack buffer happened to have zeroes in the
   right places or (2) some implementations of readlink might
   happen to NUL-terminate their result when convenient anyway.

 - it relies on PATH_MAX to determine the size of the buffer,
   so the code fails to build from source on platforms (like the
   Hurd) that have no global PATH_MAX.

Fix both by using a buffer of size strlen("<path>") + 1 and comparing
the link target to <path> with memcmp after checking that it fit in
the buffer.

For consistency with the surrounding code, the pid is considered not
to match if malloc or readlink fails.

---------------
The Hurd has most of the expected /proc/<pid> hierarchy but no
/proc/self.  Without this change, running "sleep 10 & killall sleep"
on that platform produces the confusing message:
    /proc is empty (not mounted ?)

diff --git a/ChangeLog b/ChangeLog
index b421d68152290f6ec79a4501b51c61629679db4f..c67e16f70ad3a6579ea8a079c062ddf0bfc8384a 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -12,6 +12,9 @@ Changes in 22.14
        * jiffies now ULL in killall SF#3138538
        * pstree can show parents of a process. Patch supplied by Curtis
          Hawthorne SF#3135157
+       * killall creates right size buffer instead of MAX_PATH Debian #623425
+       * check for /proc/$$/stat not /proc/self/stat which is not available
+         on hurb platforms. Same Debian bug as above
 
 Changes in 22.13
 ================

diff --git a/src/killall.c b/src/killall.c
index b3783fe6af986258e372442aab97bfe623008d1d..c14c564e4ac680f9adaf8e94757112e4d0b763fe 100644 (file)
--- a/src/killall.c
+++ b/src/killall.c
@@ -511,11 +511,14 @@ kill_all (int signal, int names, char **namelist, struct passwd *pwent)
                      /* maybe the binary has been modified and std[j].st_ino
                       * is not reliable anymore. We need to compare paths.
                       */
-                     char linkbuf[PATH_MAX];
+                     size_t len = strlen(namelist[j]);
+                     char *linkbuf = malloc(len + 1);
 
-                     if (readlink(path, linkbuf, sizeof(linkbuf)) <= 0 ||
-                                         strcmp(namelist[j], linkbuf))
+                     if (!linkbuf ||
+                         readlink(path, linkbuf, len + 1) != len ||
+                         memcmp(namelist[j], linkbuf, len))
                        ok = 0;
+                     free(linkbuf);
                    }
 
                  free(path);
@@ -683,6 +686,17 @@ void print_version()
     "For more information about these matters, see the files named COPYING.\n"));
 }
 
+static int
+have_proc_self_stat (void)
+{
+  char filename[128];
+  struct stat isproc;
+  pid_t pid = getpid();
+
+  snprintf(filename, sizeof(filename), PROC_BASE"/%d/stat", (int) pid);
+  return stat(filename, &isproc) == 0;
+}
+
 int
 main (int argc, char **argv)
 {
@@ -691,7 +705,6 @@ main (int argc, char **argv)
   int optc;
   int myoptind;
   struct passwd *pwent = NULL;
-  struct stat isproc;
   char yt[16];
   char ot[16];
 
@@ -857,8 +870,8 @@ main (int argc, char **argv)
     fprintf (stderr, _("Maximum number of names is %d\n"), MAX_NAMES);
     exit (1);
   }
-  if (stat("/proc/self/stat", &isproc)==-1) {
-    fprintf (stderr, _("%s is empty (not mounted ?)\n"), PROC_BASE);
+  if (!have_proc_self_stat()) {
+    fprintf (stderr, _("%s lacks process entries (not mounted ?)\n"), PROC_BASE);
     exit (1);
   }
   argv = argv + myoptind;