From: Craig Small <csmall@users.sourceforge.net>
Date: Sun, 19 Jun 2011 01:16:43 +0000 (+1000)
Subject: 2 Fixes for Debian Bug#623425
X-Git-Tag: v22.14~4
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=153b514abbbe76c6cff8629c99fd628f131beb4e;p=psmisc

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 b421d68..c67e16f 100644
--- 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 b3783fe..c14c564 100644
--- 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;