Date: Sun, 23 Nov 2003 12:40:13 -0800 (PST) From: Per Hedeland <per@hedeland.org> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/58803: kern.argmax isn't changeable even at boot [PATCH] Message-ID: <200311232040.hANKeDh7075312@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/58803; it has been noted by GNATS.
From: Per Hedeland <per@hedeland.org>
To: bde@zeta.org.au, freebsd-gnats-submit@freebsd.org
Cc:
Subject: Re: kern/58803: kern.argmax isn't changeable even at boot [PATCH]
Date: Sun, 23 Nov 2003 21:32:33 +0100 (CET)
Bruce Evans <bde@zeta.org.au> wrote:
>
>On Sun, 2 Nov 2003, Per Hedeland wrote:
>
>> Bruce Evans <bde@zeta.org.au> wrote:
>
>> > ...
>> >Traditionally hardwired constants cannot usefully be replaced by
>> >tunables without removing the hardwired constants. Otherwise all the
>> >users of the hardwired constants get wrong values if the tunables are
>> >actually used.
>>
>> Yes, though from a pragmatic point of view I would still think that as
>> long as this doesn't cause malfunction, it would be preferable to
>> breaking existing code that would still function with a constant but
>> incorrect definition.
>> ...
>> So, would the patch be acceptable if it also
>>
>> a) Fixed the usage of ARG_MAX in the source tree
>> b) Made the constant definition be equal to _POSIX_ARG_MAX
>> c) Prevented setting the tunable lower than _POSIX_ARG_MAX
>>
>> ? Removing the definition altogether would of course be just as simple
>> as b),
>
>That would be enough for me, except don't do (b) (leave ARG_MAX with its
>current value which is larger than _POSIX_ARG_MAX until most or all ports
>are fixed). Fixing all ports is too much to expect from anyone.
OK, I finally got around to looking at this again - an "interesting"
exercise... An updated patch is enclosed.
>Many things in the tree already use only sysconf(_SC_ARG_MAX). The
>most interesting exceptions are glob(3) (libc/gen/glob.c) and sh(1).
>glob() just uses ARG_MAX, and sh(1) doesn't use any of glob(), ARG_MAX
>or sysconf().
The glob(3) usage seems quite broken to me - it uses ARG_MAX as the
default limit on the *number* of matches when GLOB_LIMIT is set, which
is hardly meaningful. Apparently this behaviour was introduced in an
attempt make glob(3) more compatible with the NetBSD/OpenBSD
implementations, but the result is something that isn't compatible with
either the previous FreeBSD implementation or the others (which use
ARG_MAX to limit the *total size* of the matches).
Fixing that is beyond the scope for this patch I think - but since the
usage is totally unrelated to the value returned by sysconf(), I saw no
reason to put in a call to that, but simply fall back to _POSIX_ARG_MAX
if ARG_MAX isn't #defined. I applied similar reasoning to
libc/{alpha,i386}/gen/makecontext.c.
>> I just feel that it would cause unnecessary breakage. And, what
>> about NCARGS?
>
>NCARGS should have gone away when ARG_MAX became standard, so it should
>be easier to remove now. NCARGS is used mainly in tcsh. tcsh has ifdefs
>to use sysconf() if neither NCARGS nor ARG_MAX exists, but they are poorly
>implemented and just define NCARGS in terms of sysconf() so all the
>references to NCARGS in tcsh become slow.
Fixed that, and rshd/rexecd used NCARGS too. The remaining "unprotected"
usage of NCARGS/ARG_MAX was in libc/posix1e/mac.c, where it was used to
size an unused array... (already fixed in CVS).
--Per Hedeland
per@hedeland.org
=================================================================
--- /usr/src/sys/alpha/osf1/osf1_sysvec.c.ORIG Sun Sep 1 23:41:22 2002
+++ /usr/src/sys/alpha/osf1/osf1_sysvec.c Thu Oct 30 01:25:25 2003
@@ -121,7 +121,7 @@
sz = *(imgp->proc->p_sysent->sv_szsigcode);
destp = (caddr_t)arginfo - szsigcode - SPARE_USRSPACE -
- roundup((ARG_MAX - imgp->stringspace), sizeof(char *));
+ roundup((argmax - imgp->stringspace), sizeof(char *));
destp -= imgp->stringspace;
--- /usr/src/sys/ia64/ia32/ia32_sysvec.c.ORIG Sun Sep 1 23:41:23 2002
+++ /usr/src/sys/ia64/ia32/ia32_sysvec.c Thu Oct 30 01:25:25 2003
@@ -146,7 +146,7 @@
arginfo = (struct ia32_ps_strings *)IA32_PS_STRINGS;
szsigcode = *(imgp->proc->p_sysent->sv_szsigcode);
destp = (caddr_t)arginfo - szsigcode - SPARE_USRSPACE -
- roundup((ARG_MAX - imgp->stringspace), sizeof(char *));
+ roundup((argmax - imgp->stringspace), sizeof(char *));
/*
* install sigcode
@@ -194,7 +194,7 @@
/*
* Copy out strings - arguments and environment.
*/
- copyout(stringp, destp, ARG_MAX - imgp->stringspace);
+ copyout(stringp, destp, argmax - imgp->stringspace);
/*
* Fill in "ps_strings" struct for ps, w, etc.
--- /usr/src/sys/kern/kern_exec.c.ORIG Thu Dec 19 10:40:10 2002
+++ /usr/src/sys/kern/kern_exec.c Thu Oct 30 01:27:26 2003
@@ -235,7 +235,7 @@
* Allocate temporary demand zeroed space for argument and
* environment strings
*/
- imgp->stringbase = (char *)kmem_alloc_wait(exec_map, ARG_MAX +
+ imgp->stringbase = (char *)kmem_alloc_wait(exec_map, argmax +
PAGE_SIZE);
if (imgp->stringbase == NULL) {
error = ENOMEM;
@@ -243,8 +243,8 @@
goto exec_fail;
}
imgp->stringp = imgp->stringbase;
- imgp->stringspace = ARG_MAX;
- imgp->image_header = imgp->stringbase + ARG_MAX;
+ imgp->stringspace = argmax;
+ imgp->image_header = imgp->stringbase + argmax;
/*
* Translate the file name. namei() returns a vnode pointer
@@ -260,7 +260,7 @@
error = namei(ndp);
if (error) {
kmem_free_wakeup(exec_map, (vm_offset_t)imgp->stringbase,
- ARG_MAX + PAGE_SIZE);
+ argmax + PAGE_SIZE);
goto exec_fail;
}
@@ -633,7 +633,7 @@
if (imgp->stringbase != NULL)
kmem_free_wakeup(exec_map, (vm_offset_t)imgp->stringbase,
- ARG_MAX + PAGE_SIZE);
+ argmax + PAGE_SIZE);
if (imgp->object)
vm_object_deallocate(imgp->object);
@@ -987,7 +987,7 @@
if (p->p_sysent->sv_szsigcode != NULL)
szsigcode = *(p->p_sysent->sv_szsigcode);
destp = (caddr_t)arginfo - szsigcode - SPARE_USRSPACE -
- roundup((ARG_MAX - imgp->stringspace), sizeof(char *));
+ roundup((argmax - imgp->stringspace), sizeof(char *));
/*
* install sigcode
@@ -1035,7 +1035,7 @@
/*
* Copy out strings - arguments and environment.
*/
- copyout(stringp, destp, ARG_MAX - imgp->stringspace);
+ copyout(stringp, destp, argmax - imgp->stringspace);
/*
* Fill in "ps_strings" struct for ps, w, etc.
--- /usr/src/sys/kern/kern_mib.c.ORIG Fri Nov 8 00:57:17 2002
+++ /usr/src/sys/kern/kern_mib.c Thu Oct 30 01:27:27 2003
@@ -111,7 +111,7 @@
&maxusers, 0, "Hint for kernel tuning");
SYSCTL_INT(_kern, KERN_ARGMAX, argmax, CTLFLAG_RD,
- 0, ARG_MAX, "Maximum bytes of argument to execve(2)");
+ &argmax, 0, "Maximum bytes of argument to execve(2)");
SYSCTL_INT(_kern, KERN_POSIX1, posix1version, CTLFLAG_RD,
0, _POSIX_VERSION, "Version of POSIX attempting to comply to");
--- /usr/src/sys/kern/subr_param.c.ORIG Fri Aug 30 06:04:35 2002
+++ /usr/src/sys/kern/subr_param.c Sun Nov 23 19:42:30 2003
@@ -62,6 +62,12 @@
#ifndef MAXFILES
#define MAXFILES (maxproc * 2)
#endif
+#ifndef ARG_MAX
+#define ARG_MAX 65536
+#endif
+#ifndef _POSIX_ARG_MAX
+#define _POSIX_ARG_MAX 4096
+#endif
int hz;
int tick;
@@ -75,6 +81,7 @@
int nswbuf;
int maxswzone; /* max swmeta KVA storage */
int maxbcache; /* max buffer cache KVA storage */
+int argmax; /* max bytes of argument to exec */
u_quad_t maxtsiz; /* max text size */
u_quad_t dfldsiz; /* initial data size limit */
u_quad_t maxdsiz; /* max data size */
@@ -166,4 +173,9 @@
ncallout = 16 + maxproc + maxfiles;
TUNABLE_INT_FETCH("kern.ncallout", &ncallout);
+
+ argmax = ARG_MAX;
+ TUNABLE_INT_FETCH("kern.argmax", &argmax);
+ if (argmax < _POSIX_ARG_MAX)
+ argmax = _POSIX_ARG_MAX;
}
--- /usr/src/sys/sys/proc.h.ORIG Tue Dec 10 03:33:45 2002
+++ /usr/src/sys/sys/proc.h Thu Oct 30 01:27:29 2003
@@ -827,6 +827,7 @@
extern int hogticks; /* Limit on kernel cpu hogs. */
extern int nprocs, maxproc; /* Current and max number of procs. */
extern int maxprocperuid; /* Max procs per uid. */
+extern int argmax; /* Max bytes of argument to exec. */
extern u_long ps_arg_cache_limit;
extern int ps_argsopen;
extern int ps_showallprocs;
--- /usr/src/sys/vm/vm_init.c.ORIG Fri Nov 8 00:57:17 2002
+++ /usr/src/sys/vm/vm_init.c Thu Oct 30 01:25:25 2003
@@ -193,7 +193,7 @@
(nswbuf*MAXPHYS) + pager_map_size);
pager_map->system_map = 1;
exec_map = kmem_suballoc(kernel_map, &minaddr, &maxaddr,
- (16*(ARG_MAX+(PAGE_SIZE*3))));
+ (16*(argmax+(PAGE_SIZE*3))));
/*
* XXX: Mbuf system machine-specific initializations should
--- /usr/src/contrib/tcsh/ed.chared.c.ORIG Wed Jul 24 18:22:56 2002
+++ /usr/src/contrib/tcsh/ed.chared.c Sun Nov 23 14:31:58 2003
@@ -514,7 +514,7 @@
}
if (*p == '$') {
if (*++p != '-') {
- *num = NCARGS; /* Handle $ */
+ *num = ArgMax; /* Handle $ */
return(--p);
}
sign = -1; /* Handle $- */
@@ -562,13 +562,13 @@
break;
case '*':
- bend = expand_lex(buf, INBUFSIZE, &h->Hlex, 1, NCARGS);
+ bend = expand_lex(buf, INBUFSIZE, &h->Hlex, 1, ArgMax);
break;
default:
if (been_once) { /* unknown argument */
/* assume it's a modifier, e.g. !foo:h, and get whole cmd */
- bend = expand_lex(buf, INBUFSIZE, &h->Hlex, 0, NCARGS);
+ bend = expand_lex(buf, INBUFSIZE, &h->Hlex, 0, ArgMax);
q -= 2;
break;
}
@@ -661,7 +661,7 @@
}
else if (q[1] == '*') {
++q;
- to = NCARGS;
+ to = ArgMax;
}
else {
to = from;
@@ -671,7 +671,7 @@
bend = expand_lex(buf, INBUFSIZE, &h->Hlex, from, to);
}
else { /* get whole cmd */
- bend = expand_lex(buf, INBUFSIZE, &h->Hlex, 0, NCARGS);
+ bend = expand_lex(buf, INBUFSIZE, &h->Hlex, 0, ArgMax);
}
break;
}
@@ -2255,7 +2255,7 @@
hp = hp->Hnext;
if (hp == NULL) /* "can't happen" */
return(CC_ERROR);
- cp = expand_lex(hbuf, INBUFSIZE, &hp->Hlex, 0, NCARGS);
+ cp = expand_lex(hbuf, INBUFSIZE, &hp->Hlex, 0, ArgMax);
*cp = '\0';
bp = hbuf;
hp = hp->Hnext;
@@ -2277,7 +2277,7 @@
word = 0;
if (hp == NULL)
return(CC_ERROR);
- cp = expand_lex(hbuf, INBUFSIZE, &hp->Hlex, 0, NCARGS);
+ cp = expand_lex(hbuf, INBUFSIZE, &hp->Hlex, 0, ArgMax);
*cp = '\0';
bp = hbuf;
hp = hp->Hnext;
--- /usr/src/contrib/tcsh/sh.h.ORIG Wed Jul 24 18:23:01 2002
+++ /usr/src/contrib/tcsh/sh.h Sun Nov 23 15:03:33 2003
@@ -650,6 +650,7 @@
*/
EXTERN Char *doldol; /* Character pid for $$ */
EXTERN int backpid; /* pid of the last background job */
+EXTERN int ArgMax; /* Max bytes for an exec function */
/*
* Ideally these should be uid_t, gid_t, pid_t. I cannot do that right now
--- /usr/src/contrib/tcsh/tc.func.c.ORIG Wed Jul 24 18:23:04 2002
+++ /usr/src/contrib/tcsh/tc.func.c Sun Nov 23 15:33:34 2003
@@ -134,7 +134,7 @@
if (sp == (sp0 = sp0->prev))
return (buf); /* nada */
- for (i = 0; i < NCARGS; i++) {
+ for (i = 0; i < ArgMax; i++) {
if ((i >= from) && (i <= to)) { /* if in range */
for (s = sp->word; *s && d < e; s++) {
/*
@@ -174,7 +174,7 @@
{
Char *cp;
- cp = expand_lex(buf, bufsiz, sp0, 0, NCARGS);
+ cp = expand_lex(buf, bufsiz, sp0, 0, ArgMax);
*cp = '\0';
return (buf);
}
--- /usr/src/contrib/tcsh/tc.os.c.ORIG Wed Jul 24 18:23:04 2002
+++ /usr/src/contrib/tcsh/tc.os.c Sun Nov 23 15:05:10 2003
@@ -979,6 +979,24 @@
*/
syscall(151, getpid(), getpid());
#endif /* _SX */
+
+#ifndef NCARGS
+# ifdef _SC_ARG_MAX
+# define NCARGS sysconf(_SC_ARG_MAX)
+# else /* !_SC_ARG_MAX */
+# ifdef ARG_MAX
+# define NCARGS ARG_MAX
+# else /* !ARG_MAX */
+# ifdef _MINIX
+# define NCARGS 80
+# else /* !_MINIX */
+# define NCARGS 1024
+# endif /* _MINIX */
+# endif /* ARG_MAX */
+# endif /* _SC_ARG_MAX */
+#endif /* NCARGS */
+ ArgMax = NCARGS;
+
}
#ifdef strerror
--- /usr/src/contrib/tcsh/tc.os.h.ORIG Wed Jul 24 18:23:04 2002
+++ /usr/src/contrib/tcsh/tc.os.h Sun Nov 23 15:03:49 2003
@@ -105,22 +105,6 @@
# endif /* POSIX */
#endif /* OREO */
-#ifndef NCARGS
-# ifdef _SC_ARG_MAX
-# define NCARGS sysconf(_SC_ARG_MAX)
-# else /* !_SC_ARG_MAX */
-# ifdef ARG_MAX
-# define NCARGS ARG_MAX
-# else /* !ARG_MAX */
-# ifdef _MINIX
-# define NCARGS 80
-# else /* !_MINIX */
-# define NCARGS 1024
-# endif /* _MINIX */
-# endif /* ARG_MAX */
-# endif /* _SC_ARG_MAX */
-#endif /* NCARGS */
-
#ifdef convex
# include <sys/dmon.h>
#endif /* convex */
--- /usr/src/lib/libc/alpha/gen/makecontext.c.ORIG Sat Nov 16 07:39:10 2002
+++ /usr/src/lib/libc/alpha/gen/makecontext.c Sun Nov 23 21:25:31 2003
@@ -30,11 +30,19 @@
#include <sys/param.h>
#include <sys/signal.h>
+#include <limits.h>
#include <errno.h>
#include <stdarg.h>
#include <ucontext.h>
#include <unistd.h>
+/*
+ * The usage of ARG_MAX (or NCARGS) as limit on argc seems arbitrary at best
+ * - thus there is little point in finding the actual value via sysconf(3).
+ */
+#ifndef ARG_MAX
+#define ARG_MAX _POSIX_ARG_MAX
+#endif
/* Prototypes */
extern void _ctx_start(int argc, ...);
@@ -83,7 +91,7 @@
ucp->uc_mcontext.mc_format = 0;
}
/* XXX - Do we want to sanity check argc? */
- else if ((argc < 0) || (argc > NCARGS)) {
+ else if ((argc < 0) || (argc > ARG_MAX)) {
ucp->uc_mcontext.mc_format = 0;
}
/*
--- /usr/src/lib/libc/gen/glob.c.ORIG Wed Jul 17 06:58:09 2002
+++ /usr/src/lib/libc/gen/glob.c Sun Nov 23 21:14:43 2003
@@ -68,6 +68,7 @@
#include <sys/param.h>
#include <sys/stat.h>
+#include <limits.h>
#include <ctype.h>
#include <dirent.h>
#include <errno.h>
@@ -77,6 +78,15 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+
+/*
+ * The usage of ARG_MAX as default limit on the *number* of matches seems
+ * arbitrary at best - it should rather limit their total size. Given this,
+ * there is little point in finding the actual value via sysconf(3).
+ */
+#ifndef ARG_MAX
+#define ARG_MAX _POSIX_ARG_MAX
+#endif
#include "collate.h"
--- /usr/src/lib/libc/i386/gen/makecontext.c.ORIG Mon Sep 16 21:24:31 2002
+++ /usr/src/lib/libc/i386/gen/makecontext.c Sun Nov 23 21:25:41 2003
@@ -31,11 +31,20 @@
#include <sys/signal.h>
#include <sys/ucontext.h>
+#include <limits.h>
#include <errno.h>
#include <stdarg.h>
#include <stdlib.h>
#include <unistd.h>
+/*
+ * The usage of ARG_MAX (or NCARGS) as limit on argc seems arbitrary at best
+ * - thus there is little point in finding the actual value via sysconf(3).
+ */
+#ifndef ARG_MAX
+#define ARG_MAX _POSIX_ARG_MAX
+#endif
+
/* Prototypes */
extern void _ctx_start(ucontext_t *, int argc, ...);
@@ -83,7 +92,7 @@
ucp->uc_mcontext.mc_len = 0;
}
/* XXX - Do we want to sanity check argc? */
- else if ((argc < 0) || (argc > NCARGS)) {
+ else if ((argc < 0) || (argc > ARG_MAX)) {
ucp->uc_mcontext.mc_len = 0;
}
/* Make sure the context is valid. */
--- /usr/src/lib/libc/posix1e/mac.c.ORIG Tue Nov 5 02:42:35 2002
+++ /usr/src/lib/libc/posix1e/mac.c Sun Nov 23 15:16:18 2003
@@ -97,7 +97,6 @@
return (0);
while (fgets(line, LINE_MAX, file)) {
- char *argv[ARG_MAX];
char *arg, *parse, *statement, *policyname, *modulename;
int argc;
--- /usr/src/libexec/rexecd/rexecd.c.ORIG Fri May 3 15:12:06 2002
+++ /usr/src/libexec/rexecd/rexecd.c Sun Nov 23 14:24:24 2003
@@ -129,7 +129,8 @@
static void
doit(struct sockaddr *fromp)
{
- char cmdbuf[NCARGS+1], *cp;
+ char *cmdbuf, *cp;
+ int argmax;
const char *namep;
char user[16], pass[16];
struct passwd *pwd;
@@ -178,9 +179,14 @@
if (connect(sd, fromp, fromp->sa_len) < 0)
exit(1);
}
+ if ((argmax = sysconf(_SC_ARG_MAX)) == -1 ||
+ (cmdbuf = malloc(argmax + 1)) == NULL) {
+ error("%s.", strerror(errno));
+ exit(1);
+ }
getstr(user, sizeof(user), "username");
getstr(pass, sizeof(pass), "password");
- getstr(cmdbuf, sizeof(cmdbuf), "command");
+ getstr(cmdbuf, argmax + 1, "command");
(void) alarm(0);
if ((pwd = getpwnam(user)) == NULL || (pwd->pw_uid = 0 && no_uid_0) ||
@@ -207,6 +213,7 @@
}
if (pid) {
/* parent */
+ free(cmdbuf);
(void) pam_end(pamh, pam_err);
(void) close(STDIN_FILENO);
(void) close(STDOUT_FILENO);
--- /usr/src/libexec/rshd/rshd.c.ORIG Wed Jun 26 19:09:08 2002
+++ /usr/src/libexec/rshd/rshd.c Sun Nov 23 14:26:34 2003
@@ -194,7 +194,9 @@
int one = 1;
const char *cp, *errorstr;
char sig, buf[BUFSIZ];
- char cmdbuf[NCARGS+1], luser[16], ruser[16];
+ char *cmdbuf;
+ int argmax;
+ char luser[16], ruser[16];
char rhost[2 * MAXHOSTNAMELEN + 1];
char numericname[INET6_ADDRSTRLEN];
int af, srcport;
@@ -297,10 +299,14 @@
rhost[sizeof(rhost) - 1] = '\0';
/* XXX truncation! */
+ if ((argmax = sysconf(_SC_ARG_MAX)) == -1 ||
+ (cmdbuf = malloc(argmax + 1)) == NULL) {
+ rshd_errx(1, "%s.", strerror(errno));
+ }
(void) alarm(60);
getstr(ruser, sizeof(ruser), "ruser");
getstr(luser, sizeof(luser), "luser");
- getstr(cmdbuf, sizeof(cmdbuf), "command");
+ getstr(cmdbuf, argmax + 1, "command");
(void) alarm(0);
pam_err = pam_start("rsh", luser, &pamc, &pamh);
@@ -403,6 +409,7 @@
if (pid == -1)
rshd_errx(1, "Can't fork; try again.");
if (pid) {
+ free(cmdbuf);
(void) close(0);
(void) close(1);
(void) close(2);
@@ -459,6 +466,7 @@
rshd_errx(1, "Can't fork; try again.");
if (pid) {
/* Parent. */
+ free(cmdbuf);
while (wait(NULL) > 0 || errno == EINTR)
/* nothing */ ;
PAM_END;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200311232040.hANKeDh7075312>