From owner-freebsd-bugs Thu Jan 10 13:51:21 2002 Delivered-To: freebsd-bugs@hub.freebsd.org Received: from freefall.freebsd.org (freefall.FreeBSD.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id C6B0437B416 for ; Thu, 10 Jan 2002 13:50:00 -0800 (PST) Received: (from gnats@localhost) by freefall.freebsd.org (8.11.6/8.11.6) id g0ALo0W33077; Thu, 10 Jan 2002 13:50:00 -0800 (PST) (envelope-from gnats) Received: from lion.com.ua (lion.com.ua [213.133.161.130]) by hub.freebsd.org (Postfix) with ESMTP id 966B437B41C for ; Thu, 10 Jan 2002 13:49:41 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by lion.com.ua (8.11.6/8.11.6) with ESMTP id g0ALnaH08118 for ; Thu, 10 Jan 2002 23:49:37 +0200 (EET) (envelope-from sa@simon.org.ua) Message-Id: <20020110234631.Q8106-100000@lion.com.ua> Date: Thu, 10 Jan 2002 23:49:36 +0200 (EET) From: Andrey Simonenko To: FreeBSD-gnats-submit@freebsd.org Subject: bin/33774: Patch for killall(1) Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org >Number: 33774 >Category: bin >Synopsis: Patch for killall(1) >Confidential: no >Severity: non-critical >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu Jan 10 13:50:00 PST 2002 >Closed-Date: >Last-Modified: >Originator: Andrey Simonenko >Release: FreeBSD 4.4-STABLE i386 >Organization: >Environment: System: FreeBSD 4.4-STABLE i386 killall.c CVS version: $FreeBSD: src/usr.bin/killall/killall.c,v 1.5.2.4 2001/05/19 19:22:49 phk Exp $ >Description: Following patch fixes some bugs, speeds up killall(1). Bugs and parts of killall(1) which have been fixed: 1. Sometime it is impossible to use signal names without "sig" prefix and written in lower case. For example command "killall -term smth" tries to stat "/dev/ttyerm" device and command "killall -usr1 smth" tries to find user "sr1". According to killall source (line #186) case insensetive signal names are supported. 2. It is not checked if there is an argument for the -t, -u and -c options. Command line arguments parsing was re-written with getopt(3) function. So, now it is possible to use "-usr1" for the SIGUSR1 and "-u sr1" for the "sr1" user name. 3. It is possible to send signal #32 to a process (line #196), 32 is the value of the NSIG macro. Really last signal has number #31, that is NSIG-1. 4. There is some sense to use killall(1) program if at least one process name will be passed to it. It is possible to run killall(1) with some arguments, and killall(1) will do nothing (line #208). 5. If the -u option is not used and there isn't entry in the /etc/passwd file for the user, who run killall(1), then KERN_PROC_RUID isn't used in sysctl() call (lines #234-238 and #249). killall(1) can be run not only but some user, who logon on system. Also it was removed comparision for the mib[2] in the #249 line, because it is always true. 6. Speedups to lines #265-271 code. "*newprocs" variable has been removed. 7. If the -m flag is used, then all RE are compiled for every process got from the sysctl() call. So, I removed regexec() and regfree() in the loop (line #286). Instead dynamic array is allocated and all RE are compiled only once and if there is a problem with some RE, then an error message with the problem description is printed. 8. I didn't find sense of using "pmatch" variable, so I removed it. 9. Variable "matched" was removed, because whole loop (line #286) was simplified. 10. It's possible to specify incorect RE with the -m flag and in this case killall(1) "ignores" the -m flag. 11. nosig() function was removed, becase of using getopt() function and more accurate command line argument parsing. 12. usage() function was fixed, so now it displays correct arguments list. Bugs and feautures which require clarification: 1. I didn't find any good reson for the -c option, its implementation in the killall(1) source doesn't look like what is described in the killall(1) manual page. So, something (source or man-page) should be fixed. 2. Manual page says that the -m option makes arguments case insensitive RE. In the source arguments are case sensitive RE. So, something should be fixed. Comments? >How-To-Repeat: >Fix: --- /usr/src/usr.bin/killall/killall.c Sun May 20 08:06:32 2001 +++ killall.c Thu Jan 10 22:53:26 2002 @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -43,6 +44,8 @@ #include #include #include +#include +#include #include static char *prog; @@ -50,8 +53,7 @@ static void __dead2 usage(void) { - - fprintf(stderr, "usage: %s [-l] [-v] [-m] [-sig] [-u user] [-t tty] [-c cmd] [cmd]...\n", prog); + fprintf(stderr, "usage: %s [-help] [-?hlvdms] [-u user] [-t tty] [-signal] [-c procname] [procname ...]\n", prog); fprintf(stderr, "At least one option or argument to specify processes must be given.\n"); exit(1); } @@ -62,8 +64,7 @@ static char buf[80]; char *s; - strncpy(buf, str, sizeof(buf)); - buf[sizeof(buf) - 1] = '\0'; + strlcpy(buf, str, sizeof(buf)); for (s = buf; *s; s++) *s = toupper(*s); return buf; @@ -87,24 +88,15 @@ fprintf(fp, "\n"); } -static void -nosig(char *name) -{ - - warnx("unknown signal %s; valid signals:", name); - printsig(stderr); - exit(1); -} - int main(int ac, char **av) { - struct kinfo_proc *procs = NULL, *newprocs; + struct kinfo_proc *procs = NULL; struct stat sb; struct passwd *pw; - regex_t rgx; - regmatch_t pmatch; + regex_t *rgx = NULL; int i, j; + char *avopt; char buf[256]; char *user = NULL; char *tty = NULL; @@ -119,7 +111,7 @@ pid_t thispid; uid_t thisuid; dev_t thistdev; - int sig = SIGTERM; + int sig = SIGTERM, sigtmp; const char *const *p; char *ep; int errors = 0; @@ -127,85 +119,83 @@ size_t miblen; int st, nprocs; size_t size; - int matched; int killed = 0; + int opt; - prog = av[0]; - av++; - ac--; - - while (ac > 0) { - if (strcmp(*av, "-l") == 0) { - printsig(stdout); - exit(0); - } - if (strcmp(*av, "-help") == 0) + prog = basename(av[0]); + opterr = 0; + for (; optind < ac;) { + avopt = av[optind]; + if (*avopt != '-') + break; + ++avopt; + if (*avopt == '?' || strcmp(avopt, "help") == 0) usage(); - if (**av == '-') { - ++*av; - switch (**av) { - case 'u': - ++*av; - if (**av == '\0') - ++av; - --ac; - user = *av; - break; - case 't': - ++*av; - if (**av == '\0') - ++av; - --ac; - tty = *av; - break; - case 'c': - ++*av; - if (**av == '\0') - ++av; - --ac; - cmd = *av; - break; - case 'v': - vflag++; - break; - case 's': - sflag++; - break; - case 'd': - dflag++; - break; - case 'm': - mflag++; - break; - default: - if (isalpha(**av)) { - if (strncasecmp(*av, "sig", 3) == 0) - *av += 3; - for (sig = NSIG, p = sys_signame + 1; - --sig; ++p) - if (strcasecmp(*p, *av) == 0) { - sig = p - sys_signame; - break; - } - if (!sig) - nosig(*av); - } else if (isdigit(**av)) { - sig = strtol(*av, &ep, 10); - if (!*av || *ep) - errx(1, "illegal signal number: %s", *av); - if (sig < 0 || sig > NSIG) - nosig(*av); - } else - nosig(*av); + if (isalpha(*avopt)) { + if (strncasecmp(avopt, "sig", 3) == 0) + avopt += 3; + for (sigtmp = NSIG, p = sys_signame + 1; --sigtmp; ++p) + if (strcasecmp(*p, avopt) == 0) { + sigtmp = p - sys_signame; + break; + } + if (sigtmp != 0) { + ++optind; + sig = sigtmp; + continue; + } + } else if (isdigit(*avopt)) { + sigtmp = strtol(avopt, &ep, 10); + if (*ep != '\0') + errx(1, "illegal signal number: %s", avopt); + if (sigtmp > 0 && sigtmp < NSIG) { + ++optind; + sig = sigtmp; + continue; } - ++av; - --ac; - } else { + errx(1, "incorrect signal number: %d", sigtmp); + } + if ( (opt = getopt(ac, av, "dvhlmsu:t:c:")) == -1) + break; + switch (opt) { + case 'u': + user = optarg; + break; + case 't': + tty = optarg; + break; + case 'c': + cmd = optarg; + break; + case 'v': + vflag = 1; + break; + case 's': + sflag = 1; break; + case 'd': + dflag = 1; + break; + case 'm': + mflag = 1; + break; + case 'l': + printsig(stdout); + return 0; + case 'h': + usage(); + case '?': + default: + warnx("invalid switch -%c or unknown signal; valid signals:", optopt); + printsig(stderr); + return 1; } } - if (user == NULL && tty == NULL && cmd == NULL && ac == 0) + ac -= optind; + av += optind; + + if (ac == 0) usage(); if (tty) { @@ -228,18 +218,25 @@ if (pw == NULL) errx(1, "user %s does not exist", user); uid = pw->pw_uid; - if (dflag) - printf("uid:%d\n", uid); - } else { + endpwent(); + } else uid = getuid(); - if (uid != 0) { - pw = getpwuid(uid); - if (pw) - user = pw->pw_name; - if (dflag) - printf("uid:%d\n", uid); - } + + if (mflag) { + if ( (rgx = malloc((ac + 1) * sizeof *rgx)) == NULL) + err(1, "could not allocate memory"); + if (cmd != NULL) + if ( (i = regcomp(&rgx[ac], cmd, REG_EXTENDED|REG_NOSUB)) != 0) { + regerror(i, (regex_t *)NULL, buf, sizeof buf); + errx(1, "%s: illegal regexp: %s", cmd, buf); + } + for (j = 0; j < ac; j++) + if ( (i = regcomp(&rgx[j], av[j], REG_EXTENDED|REG_NOSUB)) != 0) { + regerror(i, (regex_t *)NULL, buf, sizeof buf); + errx(1, "%s: illegal regexp: %s", av[j], buf); + } } + size = 0; mib[0] = CTL_KERN; mib[1] = KERN_PROC; @@ -247,7 +244,7 @@ mib[3] = 0; miblen = 3; - if (user && mib[2] == KERN_PROC_ALL) { + if (user != NULL || uid != 0) { mib[2] = KERN_PROC_RUID; mib[3] = uid; miblen = 4; @@ -261,26 +258,21 @@ st = sysctl(mib, miblen, NULL, &size, NULL, 0); do { size += size / 10; - newprocs = realloc(procs, size); - if (newprocs == 0) { - if (procs) - free(procs); - errx(1, "could not reallocate memory"); - } - procs = newprocs; + if ( (procs = realloc(procs, size)) == NULL) + err(1, "could not reallocate memory"); st = sysctl(mib, miblen, procs, &size, NULL, 0); } while (st == -1 && errno == ENOMEM); if (st == -1) err(1, "could not sysctl(KERN_PROC)"); if (size % sizeof(struct kinfo_proc) != 0) { - fprintf(stderr, "proc size mismatch (%d total, %d chunks)\n", - size, sizeof(struct kinfo_proc)); + fprintf(stderr, "%s: proc size mismatch (%d total, %d chunks)\n", + prog, size, sizeof(struct kinfo_proc)); fprintf(stderr, "userland out of sync with kernel, recompile libkvm etc\n"); exit(1); } nprocs = size / sizeof(struct kinfo_proc); if (dflag) - printf("nprocs %d\n", nprocs); + printf("uid:%d\nnprocs %d\n", uid, nprocs); for (i = 0; i < nprocs; i++) { thispid = procs[i].kp_proc.p_pid; @@ -289,61 +281,33 @@ thistdev = procs[i].kp_eproc.e_tdev; thisuid = procs[i].kp_eproc.e_pcred.p_ruid; /* real uid */ - matched = 1; if (user) { if (thisuid != uid) - matched = 0; + continue; } if (tty) { if (thistdev != tdev) - matched = 0; + continue; } if (cmd) { if (mflag) { - if (regcomp(&rgx, cmd, - REG_EXTENDED|REG_NOSUB) != 0) { - mflag = 0; - warnx("%s: illegal regexp", cmd); - } - } - if (mflag) { - pmatch.rm_so = 0; - pmatch.rm_eo = strlen(thiscmd); - if (regexec(&rgx, thiscmd, 0, &pmatch, - REG_STARTEND) != 0) - matched = 0; - regfree(&rgx); + if (regexec(&rgx[ac], thiscmd, 0, (regmatch_t *)0, 0) != 0) + continue; } else { if (strcmp(thiscmd, cmd) != 0) - matched = 0; + continue; } } - if (matched == 0) - continue; - matched = 0; for (j = 0; j < ac; j++) { if (mflag) { - if (regcomp(&rgx, av[j], - REG_EXTENDED|REG_NOSUB) != 0) { - mflag = 0; - warnx("%s: illegal regexp", av[j]); - } - } - if (mflag) { - pmatch.rm_so = 0; - pmatch.rm_eo = strlen(thiscmd); - if (regexec(&rgx, thiscmd, 0, &pmatch, - REG_STARTEND) == 0) - matched = 1; - regfree(&rgx); + if (regexec(&rgx[j], thiscmd, 0, (regmatch_t *)0, 0) == 0) + break; } else { if (strcmp(thiscmd, av[j]) == 0) - matched = 1; + break; } - if (matched) - break; } - if (matched == 0) + if (j == ac) continue; if (dflag) printf("sig:%d, cmd:%s, pid:%d, dev:0x%x uid:%d\n", sig, @@ -353,7 +317,7 @@ printf("kill -%s %d\n", upper(sys_signame[sig]), thispid); - killed++; + killed = 1; if (!dflag && !sflag) { if (kill(thispid, sig) < 0 /* && errno != ESRCH */ ) { warn("kill -%s %d", upper(sys_signame[sig]), @@ -367,5 +331,5 @@ getuid() != 0 ? "belonging to you " : ""); errors = 1; } - exit(errors); + return errors; } >Release-Note: >Audit-Trail: >Unformatted: To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message