Source: psmisc
Version: 22.13-1
Severity: important
Tags: patch
Hi Craig,
Two quick fixes in the ongoing fight against hurd.
killall uses a buffer of size PATH_MAX (why not SYMLINK_MAX?) to
hold the link target when falling back to comparing /proc/<pid>/exe to
the <path> passed on the command line. On platforms with no
PATH_MAX, it therefore fails to build from source.
Patch 1 switches to a buffer of size strlen("<path>") + 1, which
should be large enough to fit a symlink target that matches and to
determine whether the actual target is longer. The buffer was missing
a null-terminator and tested using strcmp, so patch 1 switches to
memcmp while at it.
Patch 2 teaches killall to look at /proc/$$/stat instead of
/proc/self/stat to see whether /proc is mounted at all. Hurd supports
the former but not the latter (luckily killall doesn't seem to need
/proc/self aside from that sanity check).
So now you can use "killall foo" to kill all instances of "foo" even
on hurd.
Thoughts?
>From 0adb16446bb02c802a68c9d51f69fe61f92a646a Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <[email protected]>
Date: Tue, 19 Apr 2011 23:19:35 -0500
Subject: [PATCH 1/2] killall: fix textual path comparison fallback
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.
Signed-off-by: Jonathan Nieder <[email protected]>
---
src/killall.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/killall.c b/src/killall.c
index 8b590c2..93de30d 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);
--
1.7.5.rc2
>From efc08c926a8dbf5af4b956613dc862280e1b33df Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <[email protected]>
Date: Wed, 20 Apr 2011 00:12:00 -0500
Subject: [PATCH 2/2] killall: tolerate platforms without /proc/self
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 ?)
Afterwards, we will get
$ sleep 10 & killall sleep
[1] 9682
[1]+ Terminated sleep 10
as expected.
Signed-off-by: Jonathan Nieder <[email protected]>
---
src/killall.c | 16 +++++++++++++---
1 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/killall.c b/src/killall.c
index 93de30d..ac3a279 100644
--- a/src/killall.c
+++ b/src/killall.c
@@ -686,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)
{
@@ -694,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];
@@ -860,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;
--
1.7.5.rc2
