Message ID | 20210407151058.1176364-1-mina86@mina86.com |
---|---|
State | Committed |
Delegated to: | Adhemerval Zanella Netto |
Headers |
Return-Path: <libc-alpha-bounces@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B060C3938C17; Wed, 7 Apr 2021 15:15:58 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-ed1-x532.google.com (mail-ed1-x532.google.com [IPv6:2a00:1450:4864:20::532]) by sourceware.org (Postfix) with ESMTPS id 8289F3938C25 for <libc-alpha@sourceware.org>; Wed, 7 Apr 2021 15:15:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8289F3938C25 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=mina86.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mnazarewicz@gmail.com Received: by mail-ed1-x532.google.com with SMTP id h10so21298825edt.13 for <libc-alpha@sourceware.org>; Wed, 07 Apr 2021 08:15:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=nRq8vyIEwjkXqo1jJKYVFL21iS+novZbtWCTFby+j78=; b=QX8RZzbwTTUbeElwoqHl7U3ewlIomJvaQNYY/QVXU4nNnyg61xorYkYX3xk7M+o49J fUPpRnv7ivBIu9aA+bUKm7kiAE8CetdWnyVF3bvdGMdUHcCE9J/bGFnxUe0kfuhYFCqq LiYt1Ij148vQQYQi2t9N5MI0/UoZSzCeFlfsmWsp55898cnHAq0LD+NGCQ0Rhu0A/zfG QZUEp2RVJdTaZX/vhoVeqh1GaCWtu+qjwGrKSTfSCcCn1NJncL/o6KYOInrDYIowgc+w w3LkFWBnayENAwpAiIUkLS66tG9NQBr4iXyIljuOjpMXa6l6XCa6NLWE5QsXZq7uN2l2 FCtQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :mime-version:content-transfer-encoding; bh=nRq8vyIEwjkXqo1jJKYVFL21iS+novZbtWCTFby+j78=; b=IFHTZBIN+m6Ond5oiPkbPMPRwjjVMTqQlutN4Dk6BE4aLuqY3Zz1uUtDqNLlCrRffv h/mAhWhAfd60s49qXsDZq/KDXM5N+o6fTWxtxLHNG6TZelj7j2Opj9IRcXooUAq3NMFu M/pVlbJTs1ujIDGkSPpx0t4E9WMeexTyn5Y6U/uol09ieveaezQditAp9LW/5O+8qO8I Vr3zW6DVaa+kBdK4hFLJe4Hbl7UzTFt8Wzh1xufjZo3sUSCT1JDxW+TbjE+V/4vOddT3 we2uFvYrBCuaZNrvHQcm2EA3KLtR4Zn39BGp/GCBie3bNhq5deRZBxUjKeXfsCy6pxVv ofVg== X-Gm-Message-State: AOAM533m1e55UpzPtLSz5m/vYr98yUFYowhi3ZQru+EtFKVd26TqVIN2 AJbeO8ogEZpemEaahyu82YBkw+3Ovv/cYg== X-Google-Smtp-Source: ABdhPJy7AMNFZRIerEerdjjLAQcvz6b62u5dS1N3ClKME0kw3VU8V/arPZhl37g8Qp34lHJj88RChw== X-Received: by 2002:a05:6402:5255:: with SMTP id t21mr5075582edd.91.1617808554355; Wed, 07 Apr 2021 08:15:54 -0700 (PDT) Received: from erwin.mina86.com (77-253-94-61.adsl.inetia.pl. [77.253.94.61]) by smtp.gmail.com with ESMTPSA id k8sm487008edr.75.2021.04.07.08.15.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 07 Apr 2021 08:15:53 -0700 (PDT) From: Michal Nazarewicz <mina86@mina86.com> To: libc-alpha@sourceware.org Subject: [PATCH] linux: sysconf: limit _SC_MAX_ARG to 6 MiB [BZ #25305] Date: Wed, 7 Apr 2021 17:10:58 +0200 Message-Id: <20210407151058.1176364-1-mina86@mina86.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
linux: sysconf: limit _SC_MAX_ARG to 6 MiB [BZ #25305]
|
|
Commit Message
Michal Nazarewicz
April 7, 2021, 3:10 p.m. UTC
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 --- sysdeps/unix/sysv/linux/sysconf.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Comments
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; > } >
* Adhemerval Zanella: > 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. We can do a binary search using a test binary, perhaps the dynamic linker itself. We can probe a few known values first to speed things up, I guess. It's hideous, but given that the kernel doesn't support us here, what else can we do? Thanks, Florian
On 07/04/2021 15:41, Florian Weimer wrote: > * Adhemerval Zanella: > >> 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. > > We can do a binary search using a test binary, perhaps the dynamic > linker itself. We can probe a few known values first to speed things > up, I guess. > > It's hideous, but given that the kernel doesn't support us here, what > else can we do? IMHO being conservative and use the lower bound of all supported kernels. I really don't think trying to be smart here with dynamic probing will add much, specially since this upper limit is what kernel will support from now on.
* Adhemerval Zanella: > IMHO being conservative and use the lower bound of all supported kernels. > I really don't think trying to be smart here with dynamic probing > will add much, specially since this upper limit is what kernel will > support from now on. Ah, if the conservative choice does not penalize newer kernels, then I guess that's okay as well. Then the argument goes like this: If you want us to make the limit dynamic, add something to the auxiliary vector. 8-) Thanks, Florian
On 07/04/2021 16:04, Florian Weimer wrote: > * Adhemerval Zanella: > >> IMHO being conservative and use the lower bound of all supported kernels. >> I really don't think trying to be smart here with dynamic probing >> will add much, specially since this upper limit is what kernel will >> support from now on. > > Ah, if the conservative choice does not penalize newer kernels, then I > guess that's okay as well. My understanding is newer kernel are more restrictive since they limit maximum argument plus environment size to up to 6MB, different than older kernels that have a higher bar to up 1/4 of maximum stack size. So I take you don't oppose to the patch. > > Then the argument goes like this: If you want us to make the limit > dynamic, add something to the auxiliary vector. 8-) I am not sure if kernel will be willing to make this dynamic, at least it seems not be an issue.
* Adhemerval Zanella: > On 07/04/2021 16:04, Florian Weimer wrote: >> * Adhemerval Zanella: >> >>> IMHO being conservative and use the lower bound of all supported kernels. >>> I really don't think trying to be smart here with dynamic probing >>> will add much, specially since this upper limit is what kernel will >>> support from now on. >> >> Ah, if the conservative choice does not penalize newer kernels, then I >> guess that's okay as well. > > My understanding is newer kernel are more restrictive since they limit > maximum argument plus environment size to up to 6MB, different than older > kernels that have a higher bar to up 1/4 of maximum stack size. > > So I take you don't oppose to the patch. No, I don't opposite it. I haven't reviewed it, either. (Sorry, this week has been a mess so far.) Florian
>> * Adhemerval Zanella: >>> IMHO being conservative and use the lower bound of all supported kernels. >>> I really don't think trying to be smart here with dynamic probing >>> will add much, specially since this upper limit is what kernel will >>> support from now on. > On 07/04/2021 16:04, Florian Weimer wrote: >> Ah, if the conservative choice does not penalize newer kernels, then I >> guess that's okay as well. Indeed; the 6 MiB bound penalises old unsupported kernels. Barring Linux changing things more, the calculation in this patch is what kernel is using going forward. On Wed, Apr 07 2021, Adhemerval Zanella wrote: > My understanding is newer kernel are more restrictive since they limit > maximum argument plus environment size to up to 6MB, different than older > kernels that have a higher bar to up 1/4 of maximum stack size. Specifically, Linux’s limit is: clamp(128 KiB, max stack size / 4, 6 MiB) so stack size still plays a role but only if it’s in 0.5–24 MiB range. Outside of that range one of the static bounds is used. > So I take you don't oppose to the patch. >> Then the argument goes like this: If you want us to make the limit >> dynamic, add something to the auxiliary vector. 8-) > I am not sure if kernel will be willing to make this dynamic, at least > it seems not be an issue. I can’t read Linus’ head but I think you’re right; it seems the attitude in LKML is to use 128 KiB and be done with it.
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 Patch look good to me, thanks. Reviewed=by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > 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); I think it a bit easier to read with the value expanded (6291456). > + } > > return legacy_ARG_MAX; > } >
> 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. On Mon, Apr 12 2021, Adhemerval Zanella wrote: > Patch look good to me, thanks. > > Reviewed=by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > >> --- >> 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); > > I think it a bit easier to read with the value expanded (6291456). I’d rather go with ‘6 * 1024 * 1024’ if shifts aren’t readable. >> + } >> >> return legacy_ARG_MAX; >> } >>
On 12/04/2021 18:31, Michal Nazarewicz wrote: >> 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. > > On Mon, Apr 12 2021, Adhemerval Zanella wrote: >> Patch look good to me, thanks. >> >> Reviewed=by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >> >>> --- >>> 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); >> >> I think it a bit easier to read with the value expanded (6291456). > > I’d rather go with ‘6 * 1024 * 1024’ if shifts aren’t readable. Alright, I will change it and commit for you.
On Apr 12 2021, Michal Nazarewicz wrote: >> 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. > > On Mon, Apr 12 2021, Adhemerval Zanella wrote: >> Patch look good to me, thanks. >> >> Reviewed=by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >> >>> --- >>> 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); >> >> I think it a bit easier to read with the value expanded (6291456). > > I’d rather go with ‘6 * 1024 * 1024’ if shifts aren’t readable. In addition, I'd give it a symbolic name with a comment. Andreas.
On 13/04/2021 09:13, Andreas Schwab wrote: > On Apr 12 2021, Michal Nazarewicz wrote: > >>> 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. >> >> On Mon, Apr 12 2021, Adhemerval Zanella wrote: >>> Patch look good to me, thanks. >>> >>> Reviewed=by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >>> >>>> --- >>>> 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); >>> >>> I think it a bit easier to read with the value expanded (6291456). >> >> I’d rather go with ‘6 * 1024 * 1024’ if shifts aren’t readable. > > In addition, I'd give it a symbolic name with a comment. What about: diff --git a/sysdeps/unix/sysv/linux/sysconf.c b/sysdeps/unix/sysv/linux/sysconf.c index 366fcef01e..aceedfa87c 100644 --- a/sysdeps/unix/sysv/linux/sysconf.c +++ b/sysdeps/unix/sysv/linux/sysconf.c @@ -33,6 +33,9 @@ actual value varies based on the stack size. */ #define legacy_ARG_MAX 131072 +/* Newer kernels (4.13) limit the maximum command line arguments lengths to + 6MiB. */ +#define maximum_ARG_MAX 6291456 static long int posix_sysconf (int name); @@ -55,7 +58,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, maximum_ARG_MAX); + } return legacy_ARG_MAX; }
On Apr 13 2021, Adhemerval Zanella wrote: > On 13/04/2021 09:13, Andreas Schwab wrote: >> On Apr 12 2021, Michal Nazarewicz wrote: >> >>>> 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. >>> >>> On Mon, Apr 12 2021, Adhemerval Zanella wrote: >>>> Patch look good to me, thanks. >>>> >>>> Reviewed=by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >>>> >>>>> --- >>>>> 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); >>>> >>>> I think it a bit easier to read with the value expanded (6291456). >>> >>> I’d rather go with ‘6 * 1024 * 1024’ if shifts aren’t readable. >> >> In addition, I'd give it a symbolic name with a comment. > > What about: > > diff --git a/sysdeps/unix/sysv/linux/sysconf.c b/sysdeps/unix/sysv/linux/sysconf.c > index 366fcef01e..aceedfa87c 100644 > --- a/sysdeps/unix/sysv/linux/sysconf.c > +++ b/sysdeps/unix/sysv/linux/sysconf.c > @@ -33,6 +33,9 @@ > actual value varies based on the stack size. */ > #define legacy_ARG_MAX 131072 > > +/* Newer kernels (4.13) limit the maximum command line arguments lengths to > + 6MiB. */ > +#define maximum_ARG_MAX 6291456 I'd still use 6 * 1024 * 1024 here, small numbers are much easier to grok. Andreas.
On 13/04/2021 11:41, Andreas Schwab wrote: > On Apr 13 2021, Adhemerval Zanella wrote: > >> On 13/04/2021 09:13, Andreas Schwab wrote: >>> On Apr 12 2021, Michal Nazarewicz wrote: >>> >>>>> 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. >>>> >>>> On Mon, Apr 12 2021, Adhemerval Zanella wrote: >>>>> Patch look good to me, thanks. >>>>> >>>>> Reviewed=by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >>>>> >>>>>> --- >>>>>> 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); >>>>> >>>>> I think it a bit easier to read with the value expanded (6291456). >>>> >>>> I’d rather go with ‘6 * 1024 * 1024’ if shifts aren’t readable. >>> >>> In addition, I'd give it a symbolic name with a comment. >> >> What about: >> >> diff --git a/sysdeps/unix/sysv/linux/sysconf.c b/sysdeps/unix/sysv/linux/sysconf.c >> index 366fcef01e..aceedfa87c 100644 >> --- a/sysdeps/unix/sysv/linux/sysconf.c >> +++ b/sysdeps/unix/sysv/linux/sysconf.c >> @@ -33,6 +33,9 @@ >> actual value varies based on the stack size. */ >> #define legacy_ARG_MAX 131072 >> >> +/* Newer kernels (4.13) limit the maximum command line arguments lengths to >> + 6MiB. */ >> +#define maximum_ARG_MAX 6291456 > > I'd still use 6 * 1024 * 1024 here, small numbers are much easier to > grok. Ack.
> On 13/04/2021 09:13, Andreas Schwab wrote: >> In addition, I'd give it a symbolic name with a comment. On Tue, Apr 13 2021, Adhemerval Zanella wrote: > What about: > > diff --git a/sysdeps/unix/sysv/linux/sysconf.c b/sysdeps/unix/sysv/linux/sysconf.c > index 366fcef01e..aceedfa87c 100644 > --- a/sysdeps/unix/sysv/linux/sysconf.c > +++ b/sysdeps/unix/sysv/linux/sysconf.c > @@ -33,6 +33,9 @@ > actual value varies based on the stack size. */ > #define legacy_ARG_MAX 131072 > > +/* Newer kernels (4.13) limit the maximum command line arguments lengths to > + 6MiB. */ > +#define maximum_ARG_MAX 6291456 I’d still go with (6 * 1024 * 1024). Otherwise looks good to me. > static long int posix_sysconf (int name); > > @@ -55,7 +58,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, maximum_ARG_MAX); > + } > > return legacy_ARG_MAX; > } On Tue, Apr 13 2021, Adhemerval Zanella wrote: > Alright, I will change it and commit for you. Thanks.
On 13/04/2021 16:57, Michal Nazarewicz wrote: >> On 13/04/2021 09:13, Andreas Schwab wrote: >>> In addition, I'd give it a symbolic name with a comment. > > On Tue, Apr 13 2021, Adhemerval Zanella wrote: >> What about: >> >> diff --git a/sysdeps/unix/sysv/linux/sysconf.c b/sysdeps/unix/sysv/linux/sysconf.c >> index 366fcef01e..aceedfa87c 100644 >> --- a/sysdeps/unix/sysv/linux/sysconf.c >> +++ b/sysdeps/unix/sysv/linux/sysconf.c >> @@ -33,6 +33,9 @@ >> actual value varies based on the stack size. */ >> #define legacy_ARG_MAX 131072 >> >> +/* Newer kernels (4.13) limit the maximum command line arguments lengths to >> + 6MiB. */ >> +#define maximum_ARG_MAX 6291456 > > I’d still go with (6 * 1024 * 1024). Otherwise looks good to me. Ok, I fixed it upstream (I forgot to do it when I pushed it earlier).
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; }