Adjust maximum argument length for illumos

[citrus-it]

While using rustfd's -X option on an illumos system with a long list of files, I see:

% fd -t f . proto -X strings -a  | grep ':.*amd64' | sort
[fd error]: Problem while executing command: Arg list too long (os error 7)

This turns out to be because strings is a 32-bit program, which has a lower argument length limit than a 64-bit one.

Running this crate's tests on illumos fails:

test unix::tests::available_argument_length_is_smaller_than_experimental_limit ... FAILED

failures:

---- unix::tests::available_argument_length_is_smaller_than_experimental_limit stdout ----
Experimental limit: 1570369
thread 'unix::tests::available_argument_length_is_smaller_than_experimental_limit' panicked at 'assertion failed: available_argument_length([OsStr::new(\"echo\")].iter()).unwrap_or(0) <=\n    experimental_size_limit', src/unix.rs:110:9

sysconfig(ARG_MAX) is 2096640 for a 64-bit process and 1048320 for 32-bit.
Also defined in <limits.h>

#define _ARG_MAX32      1048320 /* max length of args to exec 32-bit program */
#define _ARG_MAX64      2096640 /* max length of args to exec 64-bit program */

The experimental limit using /usr/bin/echo (32-bit) is 1570369
The experimental limit using /usr/bin/amd64/echo (64-bit) is 2095129

I do not know a good way to retrieve the 32-bit value from the 64-bit rust process, but halving seems like a reasonable workaround.

With this fix, all crate tests pass (and rustfd patched to use this branch also works).

[tavianator]

I checked how Illumos's xargs handles this, and...

#define	MAXARGS 255

which seems a little pessimistic :)

So I think this is a fine approach, I just have the one comment.

[citrus-it]

I checked how Illumos's xargs handles this, and...

#define	MAXARGS 255

which seems a little pessimistic :)

Yes, and https://github.com/illumos/illumos-gate/blob/master/usr/src/cmd/find/find.c#L77 uses 253, both of which are too small.

So I think this is a fine approach, I just have the one comment.

Done, thanks.

[tavianator]

Hmm, actually the find implementation points something out:

  /*
   * We are in a situation where argv[0] points to a
   * script without the interpreter line, e.g. #!/bin/sh.
   * execvp() will execute either /usr/bin/sh or
   * /usr/xpg4/bin/sh against the script, and you will be
   * limited to SHELL_MAXARGS arguments. If you try to
   * pass more than SHELL_MAXARGS arguments, execvp()
   * fails with E2BIG.
   * See usr/src/lib/libc/port/gen/execvp.c.

And in execvp.c

  	for (i = 1; (newargs[i + 1] = argv[i]) != NULL; ++i) {
  		if (i >= 254) {
  			errno = E2BIG;
  			return (-1);
  		}
  	}

Which means that for the somewhat archaic case of executing a shell script without a shebang line, 255 args is really a hard limit for execvp(). You could confirm this with something like

$ cat >noshebang.sh <<EOF
# No shebang line
echo "\$@"
EOF    
$ chmod +x noshebang.sh
$ ./noshebang.sh $(yes | head -n256)

I don't think dropping the limit to 255 args on Illumos is reasonable--I think this PR is fine as-is. One possible workaround for a follow-up PR would be to resolve the executable against the PATH ourselves, and handle ENOEXEC manually with a heap-allocated argument vector.

Having said that, I think Rust might use posix_spawnp() instead of execvp() on Illumos, in which case there is no arbitrary 255-arg limit. There is, however, a potential stack overflow instead:

	/*
	 * We may need to invoke the shell with a slightly modified
	 * argv[] array.  To do this we need to preallocate the array.
	 * We must call alloca() before calling vfork() because doing
	 * it after vfork() (in the child) would corrupt the parent.
	 */
	for (argc = 0; argv[argc] != NULL; argc++)
		continue;
	newargs = alloca((argc + 2) * sizeof (char *));

This is not a security issue as long as Illumos has a stack guard page, but it's not great :/

[citrus-it]

That particular example seems ok:

bloody% uname -a
SunOS bloody 5.11 omnios-master-69de2a5279f i86pc i386 i86pc
bloody% ./noshebang.sh $(yes | head -n256)
y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y
bloody% ./noshebang.sh $(yes | head -n512)
y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y y
bloody%
bloody% head -1 noshebang.sh
# No shebang line

10455:  execve("./noshebang.sh", 0xFFFFFC7FEF010030, 0x01448A08) Err#8 ENOEXEC
10455:  execve("/bin/sh", 0xFFFFFC7FEF010028, 0x01448A08)  argc = 514

It seems like this might be worth delving more into though, thanks!

[tavianator]

Ah, well I see that Illumos's sh handles ENOEXEC itself, and probably other shells do too. If you force something to actually execvp() I think you'll see the error, maybe nice ./noshebang.sh ... or nohup ... will reproduce it.

[sharkdp]

I haven't followed the full discussion (but I can catch up if needed). What is the current state of this PR?

[tavianator]

It looks fine to me!