From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 24039CDB482 for ; Tue, 17 Oct 2023 16:59:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9705280044; Tue, 17 Oct 2023 12:59:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 920588003F; Tue, 17 Oct 2023 12:59:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8112980044; Tue, 17 Oct 2023 12:59:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 7003A8003F for ; Tue, 17 Oct 2023 12:59:57 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 2B0A980DDA for ; Tue, 17 Oct 2023 16:59:57 +0000 (UTC) X-FDA: 81355565634.01.A771BF7 Received: from mail-pf1-f170.google.com (mail-pf1-f170.google.com [209.85.210.170]) by imf01.hostedemail.com (Postfix) with ESMTP id 3AA6840018 for ; Tue, 17 Oct 2023 16:59:54 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=OpONp+Gw; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf01.hostedemail.com: domain of keescook@chromium.org designates 209.85.210.170 as permitted sender) smtp.mailfrom=keescook@chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1697561995; a=rsa-sha256; cv=none; b=2aeOjyZNERSIhXL1QWTdc0QR3925f0Cy9iBC1n0Lo2yOVNyuo4bpvNP0b4SNunnj28KuZk mAb+GQrpsehH54FN92YL55ok2efC6ddtgbY4PU59SBEA/FoHC2P1eEyaLVMryhkENsQyBZ Vtbh3dcMq3Q48zPg1L9hl/dUdSorLeM= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=OpONp+Gw; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf01.hostedemail.com: domain of keescook@chromium.org designates 209.85.210.170 as permitted sender) smtp.mailfrom=keescook@chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1697561995; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=U8F1UdIngC4vnyCLK3EqRafpf+LMFRngQaqPKzU0Jzo=; b=RX5RvomjS6SEkX6B0JhBsNoF7bNfs+9TswIuewPnnbkVa9NED7Rcf5pJmMR2jg23jt6Q79 BpVNpiEjLiPj3qsWeXTse+p6kM0OC7iGExtn3kq6O08pcIqXdu+hNFo9NLjDuzXYL874pJ z5Q9oWBSpNAoa1eWwpNed9A827y0C8Q= Received: by mail-pf1-f170.google.com with SMTP id d2e1a72fcca58-6b201a93c9cso3631338b3a.0 for ; Tue, 17 Oct 2023 09:59:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1697561994; x=1698166794; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=U8F1UdIngC4vnyCLK3EqRafpf+LMFRngQaqPKzU0Jzo=; b=OpONp+Gw0dXZ9noy+eZ5Ji42w495o10fKUavje3amK4mxtxRo7Lw9QhEYbqSqfvhm5 7386YcPqkQeSwm/gF9TKl7sDzw79ZSMilBSAf6q3M/ntBlNOzvbDCNDzgZFb2yLbrEq8 dy995umBTlxqSlR2R1n5CrfPQHHKltMJYW0iQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697561994; x=1698166794; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=U8F1UdIngC4vnyCLK3EqRafpf+LMFRngQaqPKzU0Jzo=; b=l2f1DOM8WTv/9MZeWOrBGs6rp3yGr7VAW14xSc46/aIPhh0cafz8vRDxADoPBKFH9g rLw1fLY3hlIMhcQR3ty87ETwMzipvy/QvkPCqWsCu9QM7VrQFVLUOrmW7jULo6rnA0wo fUh6iclnI2KJpSqdIUl24n0rm4lAKGFyY01Z8uwchJaMQhqFS0v0/UHY+Jmi3ESSsO6+ wAj/u+90fMtUOUEvZsu3ewR57h+bPRZwRky7Dw+dKx8WEZeSr3wz1hDZvKH2W9zr38/0 zJ16Ks8aqmVnw4tUY/h8SeQdjMq6rW2pHJU+HpSp0zCIL4rOh1LsJH3Q7I0UXgGT/plm 051Q== X-Gm-Message-State: AOJu0YwhMmMP+no1aFkdJe/TyNWbUcQ2YqBjkXg1BQgvcolOTPWAaWIg 1GdPuJ1QvloEJI2Sy2A7ZRDBMQ== X-Google-Smtp-Source: AGHT+IG6W4hgvGPyQk9HyU8a3OI89HiiQNoqDs7LZWa1NayZGlQuwkBq/thcBBgiM8kc7RQsNaeUqA== X-Received: by 2002:a05:6a21:3d85:b0:157:e4c6:766a with SMTP id bj5-20020a056a213d8500b00157e4c6766amr2419600pzc.41.1697561993806; Tue, 17 Oct 2023 09:59:53 -0700 (PDT) Received: from www.outflux.net (198-0-35-241-static.hfc.comcastbusiness.net. [198.0.35.241]) by smtp.gmail.com with ESMTPSA id c5-20020a170902c1c500b001c755810f89sm1828088plc.181.2023.10.17.09.59.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 17 Oct 2023 09:59:53 -0700 (PDT) Date: Tue, 17 Oct 2023 09:59:52 -0700 From: Kees Cook To: Sergei Trofimovich Cc: linux-kernel@vger.kernel.org, Andrew Morton , Eric Biederman , linux-mm@kvack.org Subject: Re: [RESEND PATCH] uapi: increase MAX_ARG_STRLEN from 128K to 6M Message-ID: <202310170957.DF7F7FE9FA@keescook> References: <20231016212507.131902-1-slyich@gmail.com> <202310161439.BEA904AEE8@keescook> <20231016233318.2b41ae06@nz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231016233318.2b41ae06@nz> X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 3AA6840018 X-Stat-Signature: pam1ixfgjybfpxhbee4bgurfcdbui4ca X-HE-Tag: 1697561994-64685 X-HE-Meta: U2FsdGVkX19mioBgo4DP5o1umJ3+oecn2oAVnI6o8I4l4kxz5WL9+PV18I8VCZ1mCVdt3MyfCRecim9C/eMmNmPsByf0u5FTVUEPqzRQHmRLOP5RdlHVO8oQIFHDl3+RXBgs0sYos3ulmrocAMhik2WkMiKLKSNbfYF7rRBb0zZbr/JZS0vFXBf5OUjvKRxByXSMXqF2NTLYcTzZaGQlLyhupy4jyEmIzv6gKssA6HAorjPOqPoJUVh4lX1Ut8bL/OkSHGIJd6aMaFCnVhYadqBSd0N1Pido7P4TOXwIS83ttxuQEyO+pOWNwy5KYyqPOMNTYkKjBVDfIDueI3FSbNJD3Em92NSV6E34Azjw2lWV5CmRP9UUAEdaPfQdnsWzM8yCja0wN553yjRHCmY1sNtbfg4LGManr7Nio9d/yPlwV3KDv7H9zo/2kBkK09mHh6dNb2PWuRvKzctPyiDR/VkvFKkX3ZJJ3IYtLgZamQhIrWqPO32HfBkuUPJnZZinuxyxlgQF2VLbU7GSW5GHsFrdE9lpx7ALfFpV80nQi9txoyJ6bSbhTaEbpXrew+li+At/8XXYxHOc4MenHN2MWgRS1erKh/NOSnF/V69acfvk0FtkYsaG1M6RlUN45LzkFZUaN6Vnxzw91cwnexBXezUmQHnU12HxOD9SvNbctBN2/5JtNMn0bEQ3Dmtn4WxOoRMz9foBnQQZiXf0vK+JmUT6IDmZYdELcof+ilc7/x1w4db04XjmR8QbW7WJilO10QiwibiLptR8TLAd5FRHzPQqNqu8Aa6D0piSWvRNRD/gPTySLxyuxVxGviam5mGlYXn1kw0O03ZRNF3L9Qp95PsSA3m1CXF5LdqTyfftPahyRZ1l6qQqT8LQhjI8GyNZ4YQmiQvdqYG8BB+mxOF6m0Q6sBDaOl0OSP9kb9bvZ7vfhWLQn5mnQP08yVuvlVGuDq6r87iQVMEZLJeduq6 GmHxvcnu 5NqPgwYbUYivuYwxxajJ3V9LEzoYVmqpbqOQWDF7NiN5oNdxs+bpkU+c1442zAHqnBo1AJUnxNRDEPwpbBu9Ko5uvXERnlfw4wbrZ4lILTOjv1zKLHC3A1TtTrgflaLme3fAiZ0xiCdzNR0sjtwSkvTq7bANTX2zKLFle8gejYaTcIoPzMYToh8B9aBT+KBpvt7ftZt0v+p73MdP3trvhar1EKd+e9QsEuDPmzaK0c1xncwxCUVH+KOu2SYrYRM4wENKsjdmRo7VEtn/cqD54T3FWyL9FCjuNRsMRR1AV9HjqXyzmI62/Tp7Ef4JBwuMlknUyRPlen5g17zXJw17eGYMZECyfjj8Lkqbw5W0rFLfCqv9KKAWFxhS9abXNPvToTZe0jT//pRg5tVlVY/j2jNgwUIffN+h7J3PxD3gPR00BUl3lCAZV7FY8t9s3tTWo+CoHgSXrR/gbz/DUE8hzjESWk6PsrfvZ2um/TgS8iFVVOB8ODASuatathWj73OausBxRjiP2HlRKCQYUCwlsKkHGL21RfEbRhS2aQmf3dPks1vsKhsbNIeWHZJFrhxw13Udq2wXnDusZTZDT3nz3BuEuwFdF+Avs9SgWxuWVrB1aGG2/Y6wdPw0AM6bjVtBCxeUfNPCKK6s2H0i2v+FZkW/L/px3FQuCgIuoe6G0zsXLan261bXtIlWRlPRXNOyV8cnPGuSa/SXxOfouCvL2Of9QcuLXWuT2Q0rO2DQCEu8wxhfZNVAASZ8rcF27w/8vzLNGceOghjzgAPgq0t2fGSj49T/RMC5t3JpDA5MIpVu2Guk= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Oct 16, 2023 at 11:33:18PM +0100, Sergei Trofimovich wrote: > On Mon, 16 Oct 2023 14:50:05 -0700 > Kees Cook wrote: > > > On Mon, Oct 16, 2023 at 10:25:07PM +0100, Sergei Trofimovich wrote: > > > Before the change linux allowed individual execve() arguments or > > > environment variable entries to be only as big as 32 pages. > > > > > > Histroically before b6a2fea3931 "mm: variable length argument support" > > > MAX_ARG_STRLEN used to be full allowed size `argv[] + envp[]`. > > > > > > When full limit was abandoned individual parameters were still limited > > > by a safe limit of 128K. > > > > > > Nowadays' linux allows `argv[]+envp[]` to be as laerge as 6MB (3/4 > > > `_STK_LIM`). > > > > See bprm_stack_limits() for the details, but yes, 3/4 _STK_LIM tends to > > be the max, unless the rlim_stack is set smaller. > > > > > Some build systems like `autoconf` use a single environment variable > > > to pass `CFLAGS` environment variable around. It's not a bug problem > > > if the argument list is short. > > > > > > But some packaging systems prefer installing each package into > > > individual directory. As a result that requires quite long string of > > > parameters like: > > > > > > CFLAGS="-I/path/to/pkg1 -I/path/to/pkg2 ..." > > > > > > This can easily overflow 128K and does happen for `NixOS` and `nixpkgs` > > > repositories on a regular basis. > > > > That's ... alarming. What are you doing currently to work around this? > > We usually try to stay under the threshold. When the problem arises due > to organic growth of inputs over time we either suffer build failures > without any reasonable workarounds or if the change was recent and > inflated command line options we revert the change and add hacks into > other places (like patching `gcc` directly to apply the > transformations). > > Longer term it would be nice for `gcc` to allow unbounded output via > response files, but it will take some time to sort out as current flags > rewriting expands all flags and response files into a single huge > variable and hits the 128K limit: > > https://gcc.gnu.org/pipermail/gcc/2023-October/242641.html > > Medium term dropping kernel's arbitrary small limit (this change) sounds > like the simplest solution. > > > > > > > Similar pattern is exhibited by `gcc` which converts it's input command > > > line into a single environment variable (https://gcc.gnu.org/PR111527): > > > > > > $ big_100k_var=$(printf "%0*d" 100000 0) > > > > > > # this works: 200KB of options for `printf` external command > > > $ $(which printf) "%s %s" $big_100k_var $big_100k_var >/dev/null; echo $? > > > 0 > > > > > > # this fails: 200KB of options for `gcc`, fails in `cc1` > > > $ touch a.c; gcc -c a.c -DA=$big_100k_var -DB=$big_100k_var > > > gcc: fatal error: cannot execute 'cc1': execv: Argument list too long > > > compilation terminated. > > > > > > I would say this 128KB limitation is arbitrary. > > > The change raises the limit of `MAX_ARG_STRLEN` from 32 pakes (128K > > > n `x86_64`) to the maximum limit of stack allowed by Linux today. > > > > > > It has a minor chance of overflowing userspace programs that use > > > `MAX_ARG_STRLEN` to allocate the strings on stack. It should not be a > > > big problem as such programs are already are at risk of overflowing > > > stack. > > > > > > Tested as: > > > $ V=$(printf "%*d" 1000000 0) ls > > > > > > Before the change it failed with `ls: Argument list too long`. After the > > > change `ls` executes as expected. > > > > > > WDYT of abandoning the limit and allow user to fill entire environment > > > with a single command or a single variable? > > > > Changing this value shouldn't risk any vma collisions, since exec is > > still checking the utilization before starting the actual process > > replacement steps (see bprm->argmin). > > > > It does seem pathological to set this to the full 6MB, though, since > > that would _immediately_ get rejected by execve() with an -E2BIG, but > > ultimately, there isn't really any specific limit to the length of > > individual strings as long as the entire space is less than > > bprm->argmin. > > > > Perhaps MAX_ARG_STRLEN should be removed entirely and the kernel can > > just use bprm->argmin? I haven't really looked at that to see if it's > > sane, though. It just feels like MAX_ARG_STRLEN isn't a meaningful > > limit. > > Removing the limit entirely in favour of 'bprm->argmin' sounds good. > I'll try to make it so and will send v2. > > What should be the fate of userspace-exported `MAX_ARG_STRLEN` value in > that case? Should it stay `(PAGE_SIZE * 32)`? Should it be removed > entirely? (a chance of user space build failures). > > If we are to remove it entirely what should be the reasonable limit in > kernel for other subsystems that still use it? > > fs/binfmt_elf.c: len = strnlen_user((void __user *)p, MAX_ARG_STRLEN); > fs/binfmt_elf_fdpic.c: len = strnlen_user(p, MAX_ARG_STRLEN); > fs/binfmt_flat.c: len = strnlen_user(p, MAX_ARG_STRLEN); > kernel/auditsc.c: len_full = strnlen_user(p, MAX_ARG_STRLEN) - 1; > > Keeping it at an "arbitrary" 6MB limit looked safe :) Yeah, I think leaving MAX_ARG_STRLEN totally unchanged but adjust where it is used only for the ELF loader is probably the least risky thing to do here. -Kees > > > -Kees > > > > > > > > CC: Andrew Morton > > > CC: Eric Biederman > > > CC: Kees Cook > > > CC: linux-mm@kvack.org > > > CC: linux-kernel@vger.kernel.org > > > Signed-off-by: Sergei Trofimovich > > > --- > > > include/uapi/linux/binfmts.h | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/include/uapi/linux/binfmts.h b/include/uapi/linux/binfmts.h > > > index c6f9450efc12..4e828515a22e 100644 > > > --- a/include/uapi/linux/binfmts.h > > > +++ b/include/uapi/linux/binfmts.h > > > @@ -8,11 +8,11 @@ struct pt_regs; > > > > > > /* > > > * These are the maximum length and maximum number of strings passed to the > > > - * execve() system call. MAX_ARG_STRLEN is essentially random but serves to > > > - * prevent the kernel from being unduly impacted by misaddressed pointers. > > > + * execve() system call. MAX_ARG_STRLEN is as large as Linux allows new > > > + * stack to grow. Currently it's `_STK_LIM / 4 * 3 = 6MB` (see fs/exec.c). > > > * MAX_ARG_STRINGS is chosen to fit in a signed 32-bit integer. > > > */ > > > -#define MAX_ARG_STRLEN (PAGE_SIZE * 32) > > > +#define MAX_ARG_STRLEN (6 * 1024 * 1024) > > > #define MAX_ARG_STRINGS 0x7FFFFFFF > > > > > > /* sizeof(linux_binprm->buf) */ > > > -- > > > 2.42.0 > > > > > > > -- > > Kees Cook > > > -- > > Sergei -- Kees Cook