public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: Michal Nazarewicz <mina86@mina86.com>,
	libc-alpha@sourceware.org, carlos@systemhalted.org,
	Florian Weimer <fweimer@redhat.com>
Subject: Re: [PATCH] linux: sysconf: limit _SC_MAX_ARG to 6 MiB [BZ #25305]
Date: Wed, 7 Apr 2021 15:36:48 -0300	[thread overview]
Message-ID: <6c926d3b-3094-f220-7777-91dacf975275@linaro.org> (raw)
In-Reply-To: <20210407151058.1176364-1-mina86@mina86.com>



On 07/04/2021 12:10, Michal Nazarewicz wrote:
> Since Linux 4.13, kernel limits the maximum command line arguments
> length to 6 MiB.¹  Normally the limit is still quarter of the maximum
> stack size but if that limit exceeds 6 MiB it’s clamped down.
> 
> glibc’s __sysconf implementation for Linux platform is not aware of
> this limitation and for stack sizes of over 24 MiB it returns higher
> ARG_MAX than Linux will actually accept.  This can be verified by
> executing the following application on Linux 4.13 or newer:
> 
>     #include <stdio.h>
>     #include <string.h>
>     #include <sys/resource.h>
>     #include <sys/time.h>
>     #include <unistd.h>
> 
>     int main(void) {
>             const struct rlimit rlim = { 40 * 1024 * 1024,
>                                          40 * 1024 * 1024 };
>             if (setrlimit(RLIMIT_STACK, &rlim) < 0) {
>                     perror("setrlimit: RLIMIT_STACK");
>                     return 1;
>             }
> 
>             printf("ARG_MAX     : %8ld\n", sysconf(_SC_ARG_MAX));
>             printf("63 * 100 KiB: %8ld\n", 63L * 100 * 1024);
>             printf("6 MiB       : %8ld\n", 6L * 1024 * 1024);
> 
>             char str[100 * 1024], *argv[64], *envp[1];
>             memset(&str, 'A', sizeof str);
>             str[sizeof str - 1] = '\0';
>             for (size_t i = 0; i < sizeof argv / sizeof *argv - 1; ++i) {
>                     argv[i] = str;
>             }
>             argv[sizeof argv / sizeof *argv - 1] = envp[0] = 0;
> 
>             execve("/bin/true", argv, envp);
>             perror("execve");
>             return 1;
>     }
> 
> On affected systems the program will report ARG_MAX as 10 MiB but
> despite that executing /bin/true with a bit over 6 MiB of command line
> arguments will fail with E2BIG error.  Expected result is that ARG_MAX
> is reported as 6 MiB.
> 
> Update the __sysconf function to clamp ARG_MAX value to 6 MiB if it
> would otherwise exceed it.  This resolves bug #25305 which was market
> WONTFIX as suggested solution was to cap ARG_MAX at 128 KiB.
> 
> As an aside and point of comparison, bionic (a libc implementation for
> Android systems) decided to resolve this issue by always returning 128
> KiB ignoring any potential xargs regressions.²
> 
> On older kernels this results in returning overly conservative value
> but that’s a safer option than being aggressive and returning invalid
> value on recent systems.  It’s also worth noting that at this point
> all supported Linux releases have the 6 MiB barrier so only someone
> running an unsupported kernel version would get incorrectly truncated
> result.
> 
> ¹ See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=da029c11e6b12f321f36dac8771e833b65cec962
> ² See https://android.googlesource.com/platform/bionic/+/baed51ee3a13dae4b87b11870bdf7f10bdc9efc1

It is unfortunate that Linux did not provide a dynamic way to obtain
this value, specially since it has changed over versions.  So we will
need to continue using heuristics. 

And I am not sure by what Carlos has stated in last comment from 
BZ#25305 since the only information we have is the maximum stack from
getrlimit:

| You need to implement dynamic detection of the usable value between 
| the two extreme values depending on your application requirements
| and kernel behaviour. There simply isn't a portable way to optimize 
| for the largest value.

I don't see how we can't really get the maximum upper limit with current
kernel interfaces unless we parse the kernel version and start to mimic
the kernel heuristics (which has its own issues).

And I am not sure if this characterizes as a performance regression
for process like xargs: it seems to use a conservative value for
internal arg_max (131Kb) and limits to about 2MB (1/4 of current 8MB
stack limit):

$ ./xargs/xargs --show-limits
Your environment variables take up 4599 bytes
POSIX upper limit on argument length (this system): 2090505
POSIX smallest allowable upper limit on argument length (all systems): 4096
Maximum length of command we could actually use: 2085906
Size of command buffer we are actually using: 131072
Maximum parallelism (--max-procs must be no greater): 2147483647

It would be a problem only uses start to play with -s option, but
I don't see a better way to handle.  Same for programs that 
tries to use large _SC_MAX_ARG, with current behavior they will
need to handle possible E2BIG from execve on newer kernels.

Carlos and Florian, what possible issues do you see by limiting
_SC_MAX_ARG to what kernel now expects?  IMHO this should align
with what Linux supports now.

> ---
>  sysdeps/unix/sysv/linux/sysconf.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/sysconf.c b/sysdeps/unix/sysv/linux/sysconf.c
> index 366fcef01e..bd711795c7 100644
> --- a/sysdeps/unix/sysv/linux/sysconf.c
> +++ b/sysdeps/unix/sysv/linux/sysconf.c
> @@ -55,7 +55,10 @@ __sysconf (int name)
>          struct rlimit rlimit;
>          /* Use getrlimit to get the stack limit.  */
>          if (__getrlimit (RLIMIT_STACK, &rlimit) == 0)
> -	  return MAX (legacy_ARG_MAX, rlimit.rlim_cur / 4);
> +	  {
> +	    const long int limit = MAX (legacy_ARG_MAX, rlimit.rlim_cur / 4);
> +	    return MIN (limit, 6 << 10 << 10);
> +	  }
>  
>          return legacy_ARG_MAX;
>        }
> 

  reply	other threads:[~2021-04-07 18:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-07 15:10 Michal Nazarewicz
2021-04-07 18:36 ` Adhemerval Zanella [this message]
2021-04-07 18:41   ` Florian Weimer
2021-04-07 18:52     ` Adhemerval Zanella
2021-04-07 19:04       ` Florian Weimer
2021-04-07 19:34         ` Adhemerval Zanella
2021-04-07 19:36           ` Florian Weimer
2021-04-08  1:49           ` Michal Nazarewicz
2021-04-12 20:59 ` Adhemerval Zanella
2021-04-12 21:31   ` Michal Nazarewicz
2021-04-13 11:53     ` Adhemerval Zanella
2021-04-13 12:13     ` Andreas Schwab
2021-04-13 13:36       ` Adhemerval Zanella
2021-04-13 14:41         ` Andreas Schwab
2021-04-13 14:45           ` Adhemerval Zanella
2021-04-13 19:57         ` Michal Nazarewicz
2021-04-13 20:47           ` Adhemerval Zanella

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6c926d3b-3094-f220-7777-91dacf975275@linaro.org \
    --to=adhemerval.zanella@linaro.org \
    --cc=carlos@systemhalted.org \
    --cc=fweimer@redhat.com \
    --cc=libc-alpha@sourceware.org \
    --cc=mina86@mina86.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).