Date: Sat, 13 Mar 2004 07:30:42 +0100 (CET) From: =?ISO-8859-1?Q?Magnus_B=E4ckstr=F6m?= <b@etek.chalmers.se> To: FreeBSD-gnats-submit@FreeBSD.org Subject: kern/64196: Remove the arbitrary MAXSHELLCMDLEN [PATCH] Message-ID: <20040313070129.R680@rockerduck.eep1> Resent-Message-ID: <200403130640.i2D6e9Yh052603@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 64196 >Category: kern >Synopsis: Remove the arbitrary MAXSHELLCMDLEN >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: change-request >Submitter-Id: current-users >Arrival-Date: Fri Mar 12 22:40:08 PST 2004 >Closed-Date: >Last-Modified: >Originator: Magnus Bäckström >Release: FreeBSD 5.2-CURRENT i386 >Organization: Chalmers University of Technology >Environment: System: FreeBSD rockerduck.eep1 5.2-CURRENT FreeBSD 5.2-CURRENT #3: Sat Mar 13 00:12:59 CET 2004 b@rockerduck.eep1:/foo/cur/sys.mine/i386/compile/I4100 i386 >Description: Remove the arbitrary MAXSHELLCMDLEN on the allowed length of #!/command/interpreter lines. It bites me every now and then, and is easy enough to walk around (bump the default value of 128 a little) but the limit is really rather silly and should be fixed. MAXSHELLCMDLEN's reason for existence at all is connected with the size of struct image_params, which is allocated off the kernel stack in kern_execve() in sys/kern/kern_exec.c. The patch below removes the troublesome connection with stack size, and does away with MAXSHELLCMDLEN. This allows command interpreters to be up to PAGE_SIZE long. NB: linux.ko if used should be rebuilt along with the kernel image after applying this fix. Using an old linux.ko with a patched kernel will crash the system. HEADSUP? >How-To-Repeat: Don't. :) >Fix: Index: src/lib/libc/sys/execve.2 diff -u src/lib/libc/sys/execve.2:1.2 src/lib/libc/sys/execve.2:1.2.2.1 --- src/lib/libc/sys/execve.2:1.2 Fri Mar 12 23:41:05 2004 +++ src/lib/libc/sys/execve.2 Fri Mar 12 23:42:32 2004 @@ -225,7 +225,7 @@ .It Bq Er ENAMETOOLONG When invoking an interpreted script, the interpreter name exceeds -.Dv MAXSHELLCMDLEN +.Dv PAGE_SIZE characters. .It Bq Er ENOENT The new process file does not exist. Index: src/sys/alpha/linux/linux_sysvec.c diff -u src/sys/alpha/linux/linux_sysvec.c:1.2 src/sys/alpha/linux/linux_sysvec.c:1.2.2.1 --- src/sys/alpha/linux/linux_sysvec.c:1.2 Fri Mar 12 23:50:09 2004 +++ src/sys/alpha/linux/linux_sysvec.c Fri Mar 12 23:51:44 2004 @@ -159,7 +159,7 @@ if (rpath != imgp->interpreter_name) { int len = strlen(rpath) + 1; - if (len <= MAXSHELLCMDLEN) { + if (len <= PAGE_SIZE) { memcpy(imgp->interpreter_name, rpath, len); } Index: src/sys/i386/linux/linux_sysvec.c diff -u src/sys/i386/linux/linux_sysvec.c:1.2 src/sys/i386/linux/linux_sysvec.c:1.2.2.1 --- src/sys/i386/linux/linux_sysvec.c:1.2 Fri Mar 12 20:32:28 2004 +++ src/sys/i386/linux/linux_sysvec.c Fri Mar 12 23:09:25 2004 @@ -805,7 +805,7 @@ if (rpath != imgp->interpreter_name) { int len = strlen(rpath) + 1; - if (len <= MAXSHELLCMDLEN) { + if (len <= PAGE_SIZE) { memcpy(imgp->interpreter_name, rpath, len); } free(rpath, M_TEMP); Index: src/sys/kern/imgact_shell.c diff -u src/sys/kern/imgact_shell.c:1.2 src/sys/kern/imgact_shell.c:1.2.2.4 --- src/sys/kern/imgact_shell.c:1.2 Wed Mar 10 22:25:12 2004 +++ src/sys/kern/imgact_shell.c Sat Mar 13 00:12:36 2004 @@ -28,6 +28,8 @@ __FBSDID("$FreeBSD: src/sys/kern/imgact_shell.c,v 1.26 2003/06/11 00:56:54 obrien Exp $"); #include <sys/param.h> +#include <sys/vnode.h> +#include <sys/proc.h> #include <sys/systm.h> #include <sys/sysproto.h> #include <sys/exec.h> @@ -50,7 +52,9 @@ { const char *image_header = imgp->image_header; const char *ihp, *line_endp; + struct vattr vattr; char *interp; + int error; /* a shell script? */ if (((const short *) image_header)[0] != SHELLMAGIC) @@ -68,14 +72,23 @@ /* * Copy shell name and arguments from image_header into string * buffer. + * + * At this point we have the first page of the file mapped. + * However, we don't know how far into the page the contents are + * valid -- the actual file might be much shorter than the page. + * So find out the file size. */ + error = VOP_GETATTR(imgp->vp, &vattr, imgp->proc->p_ucred, curthread); + + if (error) + return (error); - /* - * Find end of line; return if the line > MAXSHELLCMDLEN long. - */ for (ihp = &image_header[2]; *ihp != '\n' && *ihp != '#'; ++ihp) { - if (ihp >= &image_header[MAXSHELLCMDLEN]) - return(ENAMETOOLONG); + if (ihp >= &image_header[PAGE_SIZE - 1]) + return (ENAMETOOLONG); + + if (ihp >= &image_header[vattr.va_size]) + break; } line_endp = ihp; Index: src/sys/kern/kern_exec.c diff -u src/sys/kern/kern_exec.c:1.2 src/sys/kern/kern_exec.c:1.2.2.1 --- src/sys/kern/kern_exec.c:1.2 Fri Mar 12 19:13:32 2004 +++ src/sys/kern/kern_exec.c Fri Mar 12 23:09:26 2004 @@ -288,7 +288,6 @@ imgp->entry_addr = 0; imgp->vmspace_destroyed = 0; imgp->interpreted = 0; - imgp->interpreter_name[0] = '\0'; imgp->auxargs = NULL; imgp->vp = NULL; imgp->object = NULL; @@ -306,10 +305,13 @@ /* * Allocate temporary demand zeroed space for argument and - * environment strings + * environment strings. + * ARG_MAX for argument and environment, + * PAGE_SIZE for the first page of the image, + * another PAGE_SIZE for the name of interpreters. */ imgp->stringbase = (char *)kmem_alloc_wait(exec_map, ARG_MAX + - PAGE_SIZE); + PAGE_SIZE * 2); if (imgp->stringbase == NULL) { error = ENOMEM; mtx_lock(&Giant); @@ -318,6 +320,7 @@ imgp->stringp = imgp->stringbase; imgp->stringspace = ARG_MAX; imgp->image_header = imgp->stringbase + ARG_MAX; + imgp->interpreter_name = imgp->stringbase + ARG_MAX + PAGE_SIZE; /* * Translate the file name. namei() returns a vnode pointer @@ -333,7 +336,7 @@ error = namei(ndp); if (error) { kmem_free_wakeup(exec_map, (vm_offset_t)imgp->stringbase, - ARG_MAX + PAGE_SIZE); + ARG_MAX + PAGE_SIZE * 2); goto exec_fail; } @@ -704,7 +707,7 @@ if (imgp->stringbase != NULL) kmem_free_wakeup(exec_map, (vm_offset_t)imgp->stringbase, - ARG_MAX + PAGE_SIZE); + ARG_MAX + PAGE_SIZE * 2); if (imgp->object != NULL) vm_object_deallocate(imgp->object); Index: src/sys/sys/imgact.h diff -u src/sys/sys/imgact.h:1.2 src/sys/sys/imgact.h:1.2.2.1 --- src/sys/sys/imgact.h:1.2 Fri Mar 12 23:19:03 2004 +++ src/sys/sys/imgact.h Fri Mar 12 23:22:10 2004 @@ -36,8 +36,6 @@ #ifndef _SYS_IMGACT_H_ #define _SYS_IMGACT_H_ -#define MAXSHELLCMDLEN 128 - struct label; struct sysentvec; struct thread; @@ -61,7 +59,7 @@ unsigned long entry_addr; /* entry address of target executable */ char vmspace_destroyed; /* flag - we've blown away original vm space */ char interpreted; /* flag - this executable is interpreted */ - char interpreter_name[MAXSHELLCMDLEN]; /* name of the interpreter */ + char *interpreter_name; /* name of the interpreter */ void *auxargs; /* ELF Auxinfo structure pointer */ struct vm_page *firstpage; /* first page that we mapped */ char *fname; /* pointer to filename of executable (user space) */ >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040313070129.R680>