[PATCH] start-stop-daemon fails to stop processes and sometimes fails to start them.
Joakim Tjernlund
joakim.tjernlund at transmode.se
Sat Apr 19 15:04:20 UTC 2008
On Sat, 2008-04-19 at 05:13 +0200, Denys Vlasenko wrote:
> On Thursday 17 April 2008 19:02, Joakim Tjernlund wrote:
> > pid_is_cmd() would call "die" if it got a stale file.
> > readdir() will fail if a file becomes stale, detect this and
> > move on.
> >
> > This patch is aginst bb 1.8.2 gentoo version so it might not apply
> > cleanly. I hope that wont be a problem.
>
> Hrm :(
>
:) so I have bumped my patches to trunk, OK to commit?
Jocke
PS.
Please let me know if you want to approve my work beforehand,
in which case I expect you to check them in too, or if I can
commit stuff from time to time and seek approval after the fact.
>From fc4f00799fdd2e8fac12969fb71b545a7c27f93f Mon Sep 17 00:00:00 2001
From: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
Date: Sat, 19 Apr 2008 15:24:47 +0200
Subject: [PATCH] start-stop-daemon: make --exec follow symlinks
Currently start-stop-daemon --stop --exec <file> does not follow
symlinks and this is wrong.
This patch uses stat(2) to get the file's st_dev and st_ino and searches
for a matching pair. A positive side effect is the right file is always
found. The code is also smaller.
The stat(2) method is also used by debian.
Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
---
debianutils/start_stop_daemon.c | 30 +++++++++++++++---------------
1 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/debianutils/start_stop_daemon.c b/debianutils/start_stop_daemon.c
index a2679f3..5ed31d6 100644
--- a/debianutils/start_stop_daemon.c
+++ b/debianutils/start_stop_daemon.c
@@ -28,6 +28,7 @@ struct globals {
char *userspec;
char *cmdname;
char *execname;
+ struct stat execstat;
char *pidfile;
int user_id;
smallint quiet;
@@ -38,6 +39,7 @@ struct globals {
#define userspec (G.userspec )
#define cmdname (G.cmdname )
#define execname (G.execname )
+#define execstat (G.execstat )
#define pidfile (G.pidfile )
#define user_id (G.user_id )
#define quiet (G.quiet )
@@ -49,22 +51,18 @@ struct globals {
} while (0)
-static int pid_is_exec(pid_t pid, const char *name)
+static int pid_is_exec(pid_t pid, const struct stat *name_stat)
{
char buf[sizeof("/proc//exe") + sizeof(int)*3];
- char *execbuf;
- int n;
-
- sprintf(buf, "/proc/%u/exe", pid);
- n = strlen(name) + 1;
- execbuf = xzalloc(n + 1);
- readlink(buf, execbuf, n);
- /* if readlink fails because link target is longer than strlen(name),
- * execbuf still contains "", and strcmp will return !0. */
- n = strcmp(execbuf, name);
- if (ENABLE_FEATURE_CLEAN_UP)
- free(execbuf);
- return !n; /* nonzero (true) if execbuf == name */
+ struct stat st;
+
+ sprintf(buf, "/proc/%u/exe", pid);
+ if (stat(buf, &st) <0)
+ return 0;
+ if (st.st_dev == name_stat->st_dev &&
+ st.st_ino == name_stat->st_ino)
+ return 1;
+ return 0;
}
static int pid_is_user(int pid, int uid)
@@ -104,7 +102,7 @@ static void check(int pid)
{
struct pid_list *p;
- if (execname && !pid_is_exec(pid, execname)) {
+ if (execname && !pid_is_exec(pid, &execstat)) {
return;
}
if (userspec && !pid_is_user(pid, user_id)) {
@@ -300,6 +298,8 @@ int start_stop_daemon_main(int argc, char **argv)
if (errno)
user_id = xuname2uid(userspec);
}
+ if (execname)
+ stat(execname, &execstat);
if (opt & CTX_STOP) {
int i = do_stop();
--
1.5.4.5
>From e5ea707faa77f6564be646ecf33a09d7e1bf66fa Mon Sep 17 00:00:00 2001
From: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
Date: Sat, 19 Apr 2008 16:45:48 +0200
Subject: [PATCH] do_procinit() needs to check for stale directory entries.
readdir(3) will set errno when it fails to return a entry.
This will happen if a file has been deleted after the
directory has been opened.
Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
---
debianutils/start_stop_daemon.c | 16 +++++++++++-----
1 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/debianutils/start_stop_daemon.c b/debianutils/start_stop_daemon.c
index 5ed31d6..fefddb9 100644
--- a/debianutils/start_stop_daemon.c
+++ b/debianutils/start_stop_daemon.c
@@ -145,11 +145,17 @@ static void do_procinit(void)
procdir = xopendir("/proc");
pid = 0;
- while ((entry = readdir(procdir)) != NULL) {
- pid = bb_strtou(entry->d_name, NULL, 10);
- if (errno)
- continue;
- check(pid);
+ while(1) {
+ errno = 0; /* clear any previous error */
+ entry = readdir(procdir);
+ if (errno) /* Stale entry, process has died after opendir */
+ continue;
+ if (!entry) /* EOF, no more entries */
+ break;
+ pid = bb_strtou(entry->d_name, NULL, 10);
+ if (errno) /* NaN */
+ continue;
+ check(pid);
}
closedir(procdir);
if (!pid)
--
1.5.4.5
>From d41fb419c87043959c262a359921733b3323accc Mon Sep 17 00:00:00 2001
From: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
Date: Sat, 19 Apr 2008 16:52:18 +0200
Subject: [PATCH] pid_is_cmd() may not die because it cannot read a process stat entry.
Convert it to use open_read_close() and handle errors
gracefully.
Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
---
debianutils/start_stop_daemon.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/debianutils/start_stop_daemon.c b/debianutils/start_stop_daemon.c
index fefddb9..2ee2b93 100644
--- a/debianutils/start_stop_daemon.c
+++ b/debianutils/start_stop_daemon.c
@@ -76,16 +76,17 @@ static int pid_is_user(int pid, int uid)
return (sb.st_uid == uid);
}
+#define MAX_READ 1024
static int pid_is_cmd(pid_t pid, const char *name)
{
char fname[sizeof("/proc//stat") + sizeof(int)*3];
- char *buf;
+ char buf[MAX_READ+1], *p;
int r = 0;
sprintf(fname, "/proc/%u/stat", pid);
- buf = xmalloc_open_read_close(fname, NULL);
- if (buf) {
- char *p = strchr(buf, '(');
+ if (open_read_close(fname, buf, MAX_READ) > 0) {
+ buf[MAX_READ] = 0;
+ p = strchr(buf, '(');
if (p) {
char *pe = strrchr(++p, ')');
if (pe) {
@@ -93,7 +94,6 @@ static int pid_is_cmd(pid_t pid, const char *name)
r = !strcmp(p, name);
}
}
- free(buf);
}
return r;
}
--
1.5.4.5
More information about the busybox
mailing list