kern/64196: Remove the arbitrary MAXSHELLCMDLEN [PATCH]
Magnus Bäckström
b at etek.chalmers.se
Fri Mar 12 22:40:09 PST 2004
>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 at 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:
More information about the freebsd-bugs
mailing list