Date: Fri, 13 Jul 2001 10:50:33 -0400 (EDT) From: Mike Barcroft <mike@q9media.com> To: Bruce Evans <bde@zeta.org.au> Cc: audit@FreeBSD.org Subject: Re: nohup(1) enhancements patch Message-ID: <200107131450.f6DEoXF24866@coffee.q9media.com>
next in thread | raw e-mail | index | archive | help
Bruce Evans <bde@zeta.org.au> writes: > > I would appreciate comments on the patch at the end of this > > message, also available at: > > http://testbed.q9media.net/freebsd/nohup.20010713.patch > >... > > o Remove some FreeBSD specific access(2) cruft. > > This summarily blows away rev.1.2. Do you mean rev 1.3? If so, yes, access(2) suffers from a race, so you can't guarantee that your appending and not creating a new file. I even thought about doing a series of open(2)s, one without O_CREAT and one with it, but that too is a race because you can't guarentee that the file wasn't created between your first and second open(2). Better to be less discriptive, than lie to the user IMO. [snipped diff] > Non-KNF-formatted comment. > Hand-formatted comment without indent(1) protection. NetBSD and OpenBSD were both using those exact formats, but you are entirely correct. Comments fixed. [snipped diff] > Excessive vertical whitespace. All excessive vertical whitespace removed. > >... > > + execvp(argv[0], &argv[0]); > > + exit_status = (errno == ENOENT) ? EXIT_NOTFOUND : EXIT_NOEXEC; > > + err(1, "%s", argv[0]); > > + exit(exit_status); > > Last 2 lines should be "err(exit_status, argv[0]);". Good spot. Fixed. [snipped diff] > Non-KNF-formatted comment. > May violate POSIX's copyright. NetBSD and OpenBSD already have these POSIX requirements in their source trees. What's the correct solution? Contact IEEE and see if we can obtain permission, just leave it in, or pull it out? > > - if ((p = getenv("HOME"))) { > > - (void)strcpy(path, p); > > - (void)strcat(path, "/"); > > - (void)strcat(path, FILENAME); > > - append = !access(path, F_OK); > > - if ((fd = open(p = path, > > - O_RDWR|O_CREAT, S_IRUSR | S_IWUSR)) >= 0) > > + if ((p = getenv("HOME")) != NULL && *p != '\0' && > > + (strlen(p) + strlen(FILENAME) + 1) < sizeof(path)) { > > + (void)snprintf(path, sizeof(path), "%s/%s", p, FILENAME); > > Why both check that the string fits and use snprintf()? I thought it was a bit easier to read, but it isn't very useful. Reverted back to strcpy/strcat. > > + if ((fd = open(p = path, O_RDWR|O_CREAT|O_APPEND, > > + S_IRUSR|S_IWUSR)) != -1) > > Even more missing spaces arund binary operators than before. Sorry, trying to keep in sync with NetBSD and OpenBSD again. Horizontal space added. New patch at the end of this message, also available at: http://testbed.q9media.net/freebsd/nohup.20010714.patch Best regards, Mike Barcroft ---------------------------------------------------------------------- nohup.20010714.patch o Integrate security enhancements from OpenBSD. - Don't assume environment variable HOME is not NULL. o Integrate standards compliance from NetBSD. - Allow -- before the command. - Blocking SIGQUIT isn't standards compliant. - Proper exit(3) levels. o Remove some FreeBSD specific access(2) cruft. o Constify; Staticize functions; Set WARNS?=2 o Tested on i386, and alpha. Index: nohup/Makefile =================================================================== RCS file: /home/ncvs/src/usr.bin/nohup/Makefile,v retrieving revision 1.1.1.1 diff -u -r1.1.1.1 Makefile --- nohup/Makefile 1994/05/27 12:32:27 1.1.1.1 +++ nohup/Makefile 2001/07/13 14:00:51 @@ -1,5 +1,6 @@ # @(#)Makefile 8.1 (Berkeley) 6/6/93 PROG= nohup +WARNS?= 2 .include <bsd.prog.mk> Index: nohup/nohup.1 =================================================================== RCS file: /home/ncvs/src/usr.bin/nohup/nohup.1,v retrieving revision 1.8 diff -u -r1.8 nohup.1 --- nohup/nohup.1 2000/11/20 19:21:00 1.8 +++ nohup/nohup.1 2001/07/13 14:00:51 @@ -43,6 +43,7 @@ .Nd invoke a command immune to hangups .Sh SYNOPSIS .Nm +.Op Ar -- .Ar command .Op Ar arguments .Sh DESCRIPTION @@ -50,16 +51,11 @@ .Nm utility invokes .Ar command -with -its +with its .Ar arguments and at this time sets the signal .Dv SIGHUP to be ignored. -The signal -.Dv SIGQUIT -may also be set -to be ignored. If the standard output is a terminal, the standard output is appended to the file .Pa nohup.out @@ -67,10 +63,6 @@ If standard error is a terminal, it is directed to the same place as the standard output. .Pp -.Nm Nohup -exits 1 if an error occurs, otherwise the exit status is that of -.Ar command . -.Pp Some shells may provide a builtin .Nm command which is similar or identical to this utility. @@ -90,6 +82,26 @@ .Ev HOME to create the file. .El +.Sh DIAGNOSTICS +The +.Nm +utility exits with one of the following values: +.Bl -tag -width Ds +.It 126 +The +.Ar command +was found, but could not be invoked. +.It 127 +The +.Ar command +could not be found or an error occurred in +.Nm . +.El +.Pp +Otherwise, the exit status of +.Nm +will be that of +.Ar command . .Sh SEE ALSO .Xr builtin 1 , .Xr csh 1 , Index: nohup/nohup.c =================================================================== RCS file: /home/ncvs/src/usr.bin/nohup/nohup.c,v retrieving revision 1.5 diff -u -r1.5 nohup.c --- nohup/nohup.c 2000/03/26 14:46:41 1.5 +++ nohup/nohup.c 2001/07/13 14:00:51 @@ -57,67 +57,92 @@ #include <string.h> #include <unistd.h> -void dofile __P((void)); +static void dofile __P((void)); static void usage __P((void)); +#define FILENAME "nohup.out" +/* + * nohup shall exit with one of the following values: + * 126 - The utility was found, but could not be invoked. + * 127 - An error occurred in the nohup utility, or the utility could + * not be found. + */ +#define EXIT_NOEXEC 126 +#define EXIT_NOTFOUND 127 +#define EXIT_MISC 127 + int main(argc, argv) int argc; char *argv[]; { - if (argc < 2) + int exit_status; + + while (getopt(argc, argv, "") != -1) usage(); + argc -= optind; + argv += optind; + if (argc < 1) + usage(); if (isatty(STDOUT_FILENO)) dofile(); - if (isatty(STDERR_FILENO) && dup2(STDOUT_FILENO, STDERR_FILENO) == -1) { + if (isatty(STDERR_FILENO) && dup2(STDOUT_FILENO, STDERR_FILENO) == -1) /* may have just closed stderr */ - (void)fprintf(stdin, "nohup: %s\n", strerror(errno)); - exit(1); - } + err(EXIT_MISC, "%s", argv[0]); + /* The nohup utility shall take the standard action for all signals + except that SIGHUP shall be ignored. */ (void)signal(SIGHUP, SIG_IGN); - (void)signal(SIGQUIT, SIG_IGN); - execvp(argv[1], &argv[1]); - err(1, "%s", argv[1]); + execvp(argv[0], &argv[0]); + exit_status = (errno == ENOENT) ? EXIT_NOTFOUND : EXIT_NOEXEC; + err(exit_status, "%s", argv[0]); } -void +static void dofile() { - int append; int fd; - char *p, path[MAXPATHLEN]; + char path[MAXPATHLEN]; + const char *p; -#define FILENAME "nohup.out" + /* If the standard output is a terminal, all output written to + * its standard output shall be appended to the end of the file + * nohup.out in the current directory. If nohup.out cannot be + * created or opened for appending, the output shall be appended + * to the end of the file nohup.out in the directory specified + * by the HOME environment variable. + * + * If a file is created, the file's permission bits shall be + * set to S_IRUSR | S_IWUSR. + */ p = FILENAME; - append = !access(p, F_OK); - if ((fd = open(p, O_RDWR|O_CREAT, S_IRUSR | S_IWUSR)) >= 0) + fd = open(p, O_RDWR | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR); + if (fd != -1) goto dupit; - if ((p = getenv("HOME"))) { + if ((p = getenv("HOME")) != NULL && *p != '\0' && + (strlen(p) + strlen(FILENAME) + 1) < sizeof(path)) { (void)strcpy(path, p); (void)strcat(path, "/"); (void)strcat(path, FILENAME); - append = !access(path, F_OK); - if ((fd = open(p = path, - O_RDWR|O_CREAT, S_IRUSR | S_IWUSR)) >= 0) + fd = open(p = path, O_RDWR | O_CREAT | O_APPEND, + S_IRUSR | S_IWUSR); + if (fd != -1) goto dupit; } - errx(1, "can't open a nohup.out file"); + errx(EXIT_MISC, "can't open a nohup.out file"); -dupit: (void)lseek(fd, (off_t)0, SEEK_END); +dupit: + (void)lseek(fd, (off_t)0, SEEK_END); if (dup2(fd, STDOUT_FILENO) == -1) - err(1, NULL); - if (append) - (void)fprintf(stderr, "appending output to existing %s\n", p); - else - (void)fprintf(stderr, "sending output to %s\n", p); + err(EXIT_MISC, NULL); + (void)fprintf(stderr, "sending output to %s\n", p); } -void +static void usage() { - (void)fprintf(stderr, "usage: nohup command [arguments]\n"); - exit(1); + (void)fprintf(stderr, "usage: nohup [--] command [arguments]\n"); + exit(EXIT_MISC); } To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200107131450.f6DEoXF24866>