Bug 58803

Summary: [kernel] [patch] kern.argmax isn't changeable even at boot
Product: Base System Reporter: per
Component: kernAssignee: freebsd-bugs mailing list <bugs>
Status: Open ---    
Severity: Affects Only Me CC: dnelson_1901, novel, per
Priority: Normal    
Version: 5.0-RELEASE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff none

Description per 2003-11-01 12:40:16 UTC
	It isn't possible to change kern.argmax even by using the
	/boot/loader.conf etc interface. This is a rather annoying
	limitation, especially since the FreeBSD default of 64 kB could
	be considered to be pretty low (e.g. Solaris has 1 MB). Changing
	the ARG_MAX value in /usr/src/sys/sys/syslimits.h and rebuilding
	the kernel works of course, but isn't very convenient.

Fix: The trivial patch enclosed seems to fix the problem.

How-To-Repeat: 	Put a line

	kern.argmax="1048576"

	in /boot/loader.conf. Reboot, and query the new value with

	sysctl kern.argmax

	The output will still show the original 65536 value.
Comment 1 Garrett A. Wollman 2003-11-01 16:19:52 UTC
<<On Sat, 1 Nov 2003 13:36:47 +0100 (CET), Per Hedeland <per@hedeland.org> said:

> 	The trivial patch enclosed seems to fix the problem.

Your patch is incomplete, as it does not remove the definition of
ARG_MAX from <limits.h> nor fix any programs in the source tree that
use it.

-GAWollman
Comment 2 per 2003-11-01 20:13:18 UTC
Garrett Wollman <wollman@khavrinen.lcs.mit.edu> wrote:
>
><<On Sat, 1 Nov 2003 13:36:47 +0100 (CET), Per Hedeland <per@hedeland.org> said:
>
>> 	The trivial patch enclosed seems to fix the problem.
>
>Your patch is incomplete, as it does not remove the definition of
>ARG_MAX from <limits.h> nor fix any programs in the source tree that
>use it.

I find that a rather strange comment, perhaps even indicating a lack of
understanding of the general issues surrounding the replacement of
traditionally hardwired constants with tunables. Removing ARG_MAX would
obviously be quite a broken thing to do. As for the rest, <limits.h>
will bring in e.g.

#define CHILD_MAX                  40   /* max simultaneous processes */
#define OPEN_MAX                   64   /* max open files per process */

Both of these are tunables of course, run-time tunables even, and the
values given aren't even the defaults. The number of programs in the
source tree that use ARG_MAX seems to be roughly equivalent to the
number that use OPEN_MAX (very few in both cases) - apparently no-one
has seen any reason to fix the latter.

The bottom line is that any program that *really* needs to find the
*correct* value for these has to use sysconf(3) (or possibly sysctl(3))
- and a program that doesn't follow that rule is "broken" regardless of
whether the value is actually tunable or not.

It could possibly make sense to disallow the setting of kern.argmax to a
lower value than what is given via <limits.h> (and perhaps have a lower
value than the actual default there, similar to the above) - but I don't
see this being done for other tunables.

A more interesting question to my mind would be whether the patch breaks
anything in the kernel due to assumptions of ARG_MAX being a constant.

--Per Hedeland
per@hedeland.org
Comment 3 Bruce Evans 2003-11-02 07:15:56 UTC
On Sat, 1 Nov 2003, Per Hedeland wrote:

>  Garrett Wollman <wollman@khavrinen.lcs.mit.edu> wrote:
>  >
>  >Your patch is incomplete, as it does not remove the definition of
>  >ARG_MAX from <limits.h> nor fix any programs in the source tree that
>  >use it.
>
>  I find that a rather strange comment, perhaps even indicating a lack of
>  understanding of the general issues surrounding the replacement of
>  traditionally hardwired constants with tunables.

wollman is a standards lawyer, so he could have written the following
clause in POSIX.1-2001-draft7 which requires ARG_MAX to not be defined
in <limits.h> if it is a tunable:

%%%
8674               Runtime Invariant Values (Possibly Indeterminate)
8675               A definition of one of the symbolic names in the following list shall be omitted from <limits.h>
8676               on specific implementations where the corresponding value is equal to or greater than the stated
8677               minimum, but is unspecified.
8678               This indetermination might depend on the amount of available memory space on a specific
8679               instance of a specific implementation. The actual value supported by a specific instance shall be
8680               provided by the sysconf( ) function.

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.

>  Removing ARG_MAX would
>  obviously be quite a broken thing to do. As for the rest, <limits.h>
>  will bring in e.g.
>
>  #define CHILD_MAX                  40   /* max simultaneous processes */
>  #define OPEN_MAX                   64   /* max open files per process */

These and some of the other definitions in <limits.h> are longstanding bugs
in <limits.h>.

>  Both of these are tunables of course, run-time tunables even, and the
>  values given aren't even the defaults. The number of programs in the
>  source tree that use ARG_MAX seems to be roughly equivalent to the
>  number that use OPEN_MAX (very few in both cases) - apparently no-one
>  has seen any reason to fix the latter.

Actually, I fixed all uses of CHILD_MAX and OPEN_MAX in the FreeBSD
source tree almost 8 years ago.  This wasn't very hard, because CHILD_MAX
is mostly unused, and most places that want the number of open files
already used getdtablesize(3).  I removed their definitions from
<limits.h> at the same time.  Committing this part got sidetracked.
It is even harder to fix now, since there probably many ports than use
OPEN_MAX even when it is not defined.  The removal of OPEN_MAX in the
FreeBSD source tree has only been broken in doscmd.

>  The bottom line is that any program that *really* needs to find the
>  *correct* value for these has to use sysconf(3) (or possibly sysctl(3))
>  - and a program that doesn't follow that rule is "broken" regardless of
>  whether the value is actually tunable or not.

Such programs are only broken on on systems that don't define the
limits.  Definition of a limit as a constant means that the limit is
actually constant.  Programs are entitled to expect that the limits
work as specified if they are defined.

As a practical matter, I would never use the #defined limits except
for things that are specified to be constant (like INT_MAX), since
supporting the special cases where the limits happen to be compile-time
constants mainly just takes more code.  I'm familiar with some POSIX
conformance tests that take an enormous amount of code to handle all
the possible cases.

>  It could possibly make sense to disallow the setting of kern.argmax to a
>  lower value than what is given via <limits.h> (and perhaps have a lower
>  value than the actual default there, similar to the above) - but I don't
>  see this being done for other tunables.

POSIX also requires {ARG_MAX} >= _POSIX_ARG_MAX && __POSIX_ARG_MAX == 4096,
so there must be a lower limit of 4096 on any tunable.  Indeed, this stuff
is quite broken for other tunables, sysctls and setrlimit() calls that
can reduce the limits below their POSIX minimums or below ones that work.

Bruce
Comment 4 per 2003-11-02 11:12:08 UTC
Bruce Evans <bde@zeta.org.au> wrote:
>
>On Sat, 1 Nov 2003, Per Hedeland wrote:
>
>>  Garrett Wollman <wollman@khavrinen.lcs.mit.edu> wrote:
>>  >
>>  >Your patch is incomplete, as it does not remove the definition of
>>  >ARG_MAX from <limits.h> nor fix any programs in the source tree that
>>  >use it.
>>
>>  I find that a rather strange comment, perhaps even indicating a lack of
>>  understanding of the general issues surrounding the replacement of
>>  traditionally hardwired constants with tunables.
>
>wollman is a standards lawyer, so he could have written the following
>clause in POSIX.1-2001-draft7 which requires ARG_MAX to not be defined
>in <limits.h> if it is a tunable:
>
>%%%
>8674               Runtime Invariant Values (Possibly Indeterminate)
>8675               A definition of one of the symbolic names in the following list shall be omitted from <limits.h>
>8676               on specific implementations where the corresponding value is equal to or greater than the stated
>8677               minimum, but is unspecified.
>8678               This indetermination might depend on the amount of available memory space on a specific
>8679               instance of a specific implementation. The actual value supported by a specific instance shall be
>8680               provided by the sysconf( ) function.

Many thanks for your detailed explanation, and my apologies to Garrett
Wollman for jumping to conclusions based on the existing FreeBSD code
and the assumption that adhering to POSIX wouldn't break existing code
(I didn't have the spec at hand).

>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.

I realize that at least in the case of ARG_MAX, a program could expect
that all its arguments and environment strings would fit in an array of
size ARG_MAX and thus break badly if this wasn't the case. However I
would expect that "typical" usage would be for sizing an array for
arguments given to a forked/executed program, which wouldn't break as
long as the constant is lower than the actual value.

>Actually, I fixed all uses of CHILD_MAX and OPEN_MAX in the FreeBSD
>source tree almost 8 years ago.  This wasn't very hard, because CHILD_MAX
>is mostly unused, and most places that want the number of open files
>already used getdtablesize(3).  I removed their definitions from
><limits.h> at the same time.  Committing this part got sidetracked.
>It is even harder to fix now, since there probably many ports than use
>OPEN_MAX even when it is not defined.  The removal of OPEN_MAX in the
>FreeBSD source tree has only been broken in doscmd.

OK - removing the use of ARG_MAX without a guarding #ifdef in the source
tree of the base system seems to be pretty trivial, but of course it has
the same problem (unfixable IMHO) visavi the ports. So what is the
conclusion of this? Can we never make ARG_MAX a tunable, or should we
take the same pragmatic but non-POSIX-compliant approach as is
effectively done for OPEN_MAX?

>>  It could possibly make sense to disallow the setting of kern.argmax to a
>>  lower value than what is given via <limits.h> (and perhaps have a lower
>>  value than the actual default there, similar to the above) - but I don't
>>  see this being done for other tunables.
>
>POSIX also requires {ARG_MAX} >= _POSIX_ARG_MAX && __POSIX_ARG_MAX == 4096,
>so there must be a lower limit of 4096 on any tunable.  Indeed, this stuff
>is quite broken for other tunables, sysctls and setrlimit() calls that
>can reduce the limits below their POSIX minimums or below ones that work.

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), I just feel that it would cause unnecessary breakage. And, what
about NCARGS?

Thanks
--Per Hedeland
per@hedeland.org
Comment 5 Bruce Evans 2003-11-03 10:39:45 UTC
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.

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().

> 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.

BTW, I've thought a bit of removing the ARG_MAX limit almost completely.
Premptive kernels should prevent denial of service attacks in the time
domain the would result from processing almost infinitely args in a
non-preemptible execve() call.  But a limit on space is still needed,
since the execve() wants to kmem_alloc() the space and kernel memory
is limited.

Bruce
Comment 6 per 2003-11-23 20:32:33 UTC
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;
Comment 7 Bruce Evans 2003-11-27 10:02:24 UTC
On Sun, 23 Nov 2003, Per Hedeland wrote:

> Bruce Evans <bde@zeta.org.au> wrote:
> >
> >On Sun, 2 Nov 2003, Per Hedeland wrote:
> >> ...
> >> 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.

This seems to be along the right lines.  I hope to commit it (with some
style changes) the release freeze.  (Actually, I hope someone else will
do this :-).

Someone pointed to a similar problem with NGROUPS_MAX and NGROUPS.  It
seems to be easier to fix.

Bruce
Comment 8 Mark Linimon 2012-01-19 17:10:41 UTC
----- Forwarded message from John Kozubik <john@kozubik.com> -----

Date: Fri, 13 Jan 2012 09:16:23 -0800 (PST)
From: John Kozubik <john@kozubik.com>
To: freebsd-bugs@freebsd.org
Cc: bde@zeta.org.au
Subject: kern/58803 [kernel][patch][OLD] - fix available, but no commit ...
	(was: kern.argmax isn't changeable even at boot)
User-Agent: Alpine 2.00 (BSF 1167 2008-08-23)

Hello,

kern/58803 is quite old, but appears to have a workable patch ... it
would be nice to get this into place.

I have confirmed that it is still impossible to set a kern.argmax
value larger than default using loader.conf on 8.2-RELEASE.

Thanks.
_______________________________________________
freebsd-bugs@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "freebsd-bugs-unsubscribe@freebsd.org"


----- End forwarded message -----
Comment 9 Mark Linimon 2012-07-16 03:33:35 UTC
----- Forwarded message from John Kozubik <john@kozubik.com> -----

Date: Mon, 25 Jun 2012 09:55:28 -0700 (PDT)
From: John Kozubik <john@kozubik.com>
To: freebsd-bugs@freebsd.org
Subject: kern/58803 - [kernel][patch][NINE-YEARS-OLD] - fix available, but
	no commit ...

Friends,


# uname -a
FreeBSD test.rsync.net 8.3-RELEASE FreeBSD 8.3-RELEASE #0: Fri Jun 22
12:50:00 PDT 2012     root@:/usr/src/sys/amd64/compile/test  amd64
#
# cat /boot/loader.conf|grep argmax
kern.argmax="1048576"
#
# sysctl -a|grep argmax
kern.argmax: 262144
#
#

It's nice to see the larger default in place, as opposed to 65536, but
is it not possible to fix this so that it can actually be tuned ?

Thanks.
_______________________________________________
freebsd-bugs@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "freebsd-bugs-unsubscribe@freebsd.org"


----- End forwarded message -----
Comment 10 Roman Bogorodskiy freebsd_committer 2016-06-27 22:27:00 UTC
I've updated this patch for fresh -CURRENT and created a review request:

https://reviews.freebsd.org/D6999

Reviews and suggestions are more than welcome.
Comment 11 Eitan Adler freebsd_committer freebsd_triage 2018-05-20 23:52:11 UTC
For bugs matching the following conditions:
- Status == In Progress
- Assignee == "bugs@FreeBSD.org"
- Last Modified Year <= 2017

Do
- Set Status to "Open"