Date: Tue, 21 Sep 2010 16:24:51 +0000 (UTC) From: Alan Cox <alc@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r212965 - in head: lib/libc/sys sys/kern sys/sys Message-ID: <201009211624.o8LGOpZc099335@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: alc Date: Tue Sep 21 16:24:51 2010 New Revision: 212965 URL: http://svn.freebsd.org/changeset/base/212965 Log: Fix exec_imgact_shell()'s handling of two error cases: (1) Previously, if the first line of a script exceeded MAXSHELLCMDLEN characters, then exec_imgact_shell() silently truncated the line and passed on the truncated interpreter name or argument. Now, exec_imgact_shell() will fail and return ENOEXEC, which is the commonly used errno among Unix variants for this type of error. (2) Previously, exec_imgact_shell()'s check on the length of the interpreter's name was ineffective. In other words, exec_imgact_shell() could not possibly fail and return ENAMETOOLONG. The reason being that the length of the interpreter name had to exceed MAXSHELLCMDLEN characters in order that ENAMETOOLONG be returned. But, the search for the end of the interpreter name stops after at most MAXSHELLCMDLEN - 2 characters are scanned. (In the end, this particular error is eventually discovered outside of exec_imgact_shell() and ENAMETOOLONG is returned. So, the real effect of this second change is that the error is detected earlier, in exec_imgact_shell().) Update the definition of MAXINTERP to the actual limit on the size of the interpreter name that has been in effect since r142453 (from 2005). In collaboration with: kib Modified: head/lib/libc/sys/execve.2 head/sys/kern/imgact_shell.c head/sys/sys/param.h Modified: head/lib/libc/sys/execve.2 ============================================================================== --- head/lib/libc/sys/execve.2 Tue Sep 21 15:07:44 2010 (r212964) +++ head/lib/libc/sys/execve.2 Tue Sep 21 16:24:51 2010 (r212965) @@ -28,7 +28,7 @@ .\" @(#)execve.2 8.5 (Berkeley) 6/1/94 .\" $FreeBSD$ .\" -.Dd April 10, 2008 +.Dd September 21, 2010 .Dt EXECVE 2 .Os .Sh NAME @@ -256,9 +256,11 @@ A component of the path prefix is not a .It Bq Er ENAMETOOLONG A component of a pathname exceeded 255 characters, or an entire path name exceeded 1023 characters. -.It Bq Er ENAMETOOLONG -When invoking an interpreted script, the interpreter name -exceeds +.It Bq Er ENOEXEC +When invoking an interpreted script, the length of the first line, +inclusive of the +.Sy \&#! +prefix and terminating newline, exceeds .Dv MAXSHELLCMDLEN characters. .It Bq Er ENOENT Modified: head/sys/kern/imgact_shell.c ============================================================================== --- head/sys/kern/imgact_shell.c Tue Sep 21 15:07:44 2010 (r212964) +++ head/sys/kern/imgact_shell.c Tue Sep 21 16:24:51 2010 (r212965) @@ -46,13 +46,18 @@ __FBSDID("$FreeBSD$"); /* * At the time of this writing, MAXSHELLCMDLEN == PAGE_SIZE. This is * significant because the caller has only mapped in one page of the - * file we're reading. This code should be changed to know how to - * read in the second page, but I'm not doing that just yet... + * file we're reading. */ #if MAXSHELLCMDLEN > PAGE_SIZE #error "MAXSHELLCMDLEN is larger than a single page!" #endif +/* + * MAXSHELLCMDLEN must be at least MAXINTERP plus the size of the `#!' + * prefix and terminating newline. + */ +CTASSERT(MAXSHELLCMDLEN >= MAXINTERP + 3); + /** * Shell interpreter image activator. An interpreter name beginning at * imgp->args->begin_argv is the minimal successful exit requirement. @@ -98,20 +103,20 @@ exec_shell_imgact(imgp) const char *image_header = imgp->image_header; const char *ihp, *interpb, *interpe, *maxp, *optb, *opte, *fname; int error, offset; - size_t length, clength; + size_t length; struct vattr vattr; struct sbuf *sname; /* a shell script? */ - if (((const short *) image_header)[0] != SHELLMAGIC) - return(-1); + if (((const short *)image_header)[0] != SHELLMAGIC) + return (-1); /* * Don't allow a shell script to be the shell for a shell * script. :-) */ if (imgp->interpreted) - return(ENOEXEC); + return (ENOEXEC); imgp->interpreted = 1; @@ -127,12 +132,9 @@ exec_shell_imgact(imgp) /* * Copy shell name and arguments from image_header into a string - * buffer. Remember that the caller has mapped only the - * first page of the file into memory. + * buffer. */ - clength = (vattr.va_size > PAGE_SIZE) ? PAGE_SIZE : vattr.va_size; - - maxp = &image_header[clength]; + maxp = &image_header[MIN(vattr.va_size, MAXSHELLCMDLEN)]; ihp = &image_header[2]; /* @@ -149,7 +151,7 @@ exec_shell_imgact(imgp) interpe = ihp; if (interpb == interpe) return (ENOEXEC); - if ((interpe - interpb) >= MAXSHELLCMDLEN) + if (interpe - interpb >= MAXINTERP) return (ENAMETOOLONG); /* @@ -163,6 +165,8 @@ exec_shell_imgact(imgp) while (ihp < maxp && ((*ihp != '\n') && (*ihp != '\0'))) ihp++; opte = ihp; + if (opte == maxp) + return (ENOEXEC); while (--ihp > optb && ((*ihp == ' ') || (*ihp == '\t'))) opte = ihp; Modified: head/sys/sys/param.h ============================================================================== --- head/sys/sys/param.h Tue Sep 21 15:07:44 2010 (r212964) +++ head/sys/sys/param.h Tue Sep 21 16:24:51 2010 (r212965) @@ -73,7 +73,7 @@ #include <sys/syslimits.h> #define MAXCOMLEN 19 /* max command name remembered */ -#define MAXINTERP 32 /* max interpreter file name length */ +#define MAXINTERP PATH_MAX /* max interpreter file name length */ #define MAXLOGNAME 17 /* max login name length (incl. NUL) */ #define MAXUPRC CHILD_MAX /* max simultaneous processes */ #define NCARGS ARG_MAX /* max bytes for an exec function */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201009211624.o8LGOpZc099335>