Notes |
(0009134)
vda
07-10-08 16:41
|
Before trying to fix it, we need to understand how exactly it fails.
Does it happen with 1.11.0? For me it works:
# strace -o log killall top
# tail log
read(5, "32314 (mc) S 32312 32314 32312 3"..., 1023) = 244
close(5) = 0
open("/proc/32314/cmdline", O_RDONLY|O_LARGEFILE) = 5
read(5, "/usr/bin/mc\0-d\0", 1023) = 15
close(5) = 0
getdents64(3, /* 0 entries */, 1024) = 0
close(3) = 0
kill(707, SIGTERM) = 0
kill(14022, SIGTERM) = 0
_exit(0) = ?
Can you provide your example? Attach full strace log to this bug. |
| |
(0009144)
bwondu
07-10-08 21:00
|
open("/proc/25936/cmdline", O_RDONLY|O_LARGEFILE) = 4
read(4, "./busybox\0killall\0top\0", 1023) = 22
close(4) = 0
getdents64(3, /* 0 entries */, 1024) = 0
close(3) = 0
brk(0x8aad000) = 0x8aad000
ioctl(2147483647, SNDCTL_TMR_TIMEBASE or TCGETS, 0xbfe39758) = -1 EBADF (Bad file descriptor)
brk(0x8aae000) = 0x8aae000
write(2, "killall: top: no process killed\n", 32) = 32
_exit(1) = ? |
| |
(0009154)
vda
07-11-08 03:08
|
This is not a full strace log. Instead of pasting "tail log" result, push [Upload File] button on this web page and upload _entire_ strace log file. Please also specify the name of process you are trying to kill - "killall <WHICH_PROGRAM?>". |
| |
(0009164)
bwondu
07-11-08 03:37
|
I have uploaded a full tracelog. Please refer to stracelog. I tried to kill nonsenseprocess.sh |
| |
(0009174)
vda
07-11-08 03:59
|
Here is it:
open("/proc/32635/stat", O_RDONLY|O_LARGEFILE) = 4
read(4, "32635 (nonsenseprocess) S 32604 "..., 1023) = 200
close(4) = 0
The _ONLY_ way to match scripts by name in Linux is to look at so called comm field in /proc/$PID/stat.
Because /proc/$PID/cmdline will have "/bin/sh",NUL,"<scriptname>",NUL,... and /proc/$PID/exe symlink will point to /bin/sh, and neither of these are indicative that this process is named <scriptname>, right?
So we are left with /proc/$PID/stat's comm field - the "...(<scriptname>)..." thing. But it is limited to 15 chars. If killall sees comm field which is exactly 15 chars long, it does not use it because it is truncated - in your example, it can be a truncated version of "nonsenseprocess.sh" or truncated version of "nonsenseprocess_which_you_dont_want_to_kill_at_all_costs"!
Killall should be able to kill scripts with names less than 15 chars. |
| |
(0009234)
bwondu
07-13-08 18:17
|
Since we know how many bytes were read from the /proc/%d/cmd file, we can change the null characters to non-null values (spaces). If you use that file, there is no need to worry about truncation.
My 2 cents. |
| |
(0009244)
vda
07-13-08 21:15
|
You misunderstood me. /proc/$PID/cmdline looks like this (after replacing NULs with spaces):
"/bin/sh nonsenseprocess.sh [param]..."
How can possibly "killall nonsenseprocess.sh" figure out that a process with such cmdline is supposed to be killed? It's not much different from, say,
"/bin/vi nonsenseprocess.sh [param]..."
and *this* process is definitely should not be killed by "killall nonsenseprocess.sh", right?
Is killall supposed to treat "/bin/sh" specially in cmdline? I don't think so. |
| |
(0009254)
bwondu
07-14-08 07:45
|
I understand that the arguments given on the command line ([param]) put a wrench in things. I suggested it as a starting point. But maybe a combination of the /proc/$PID/cmdline file and /proc/$PID/stat file would work better.
if( strstr(stringFromcmdline, procToKill) != NULL && strstr(procToKill, scriptnameFromStat) != NULL) or something like that |
| |
(0009264)
vda
07-14-08 10:43
|
Optional parameters is not a problem. The problem is that script name is not in argv[0], it's in argv[1], and it is dangerous for killall to assume that if argv[1] == program_to_kill, it should kill this process.
> if( strstr(stringFromcmdline, procToKill) != NULL && strstr(procToKill, scriptnameFromStat) != NULL)
Hmm. Maybe. I'd like to be extra paranoid in such cases, though... maybe this?
if (strcmp(cmdline[0], procToKill)==0)
kill(pid);
else
if (strcmp(cmdline[1], procToKill)==0
&& strncmp(comm, procToKill, 15)==0
&& well_known_interpreter(cmdline[0])
) {
kill(pid);
}
How would you define well_known_interpreter() function? |
| |
(0009484)
vda
07-16-08 16:19
|
Please try attached 8.patch |
| |
(0009634)
bwondu
07-17-08 10:08
|
The patch failed. I tried it on a pristine extract from a busybox-1.9.1.tar.bz2 package I downloaded on Feb 12.
patch -p0 < 8.patch
patching file busybox.8/include/libbb.h
Hunk 0000001 succeeded at 978 (offset -188 lines).
Hunk 2 succeeded at 1213 with fuzz 2 (offset -1 lines).
patching file busybox.8/libbb/find_pid_by_name.c
Hunk 2 FAILED at 77.
1 out of 2 hunks FAILED -- saving rejects to file busybox.8/libbb/find_pid_by_name.c.rej
patching file busybox.8/libbb/procps.c
Reversed (or previously applied) patch detected! Assume -R? [n] |
| |
(0009644)
vda
07-17-08 11:00
|
Latest release is 1.11.1, I worked on it. Sorry. |
| |
(0009734)
bernhardf
07-18-08 00:10
|
I suggest you try to apply it to current svn. |
| |
(0011754)
vda
09-20-08 13:28
|
Fixed in svn. |
| |
log [^] (22,959 bytes) 07-10-08 21:01
