2023-09-24 22:30:23

by Sergei Trofimovich

[permalink] [raw]
Subject: [PATCH] uapi: increase MAX_ARG_STRLEN from 128K to 6M

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

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.

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?

CC: Eric Biederman <[email protected]>
CC: Kees Cook <[email protected]>
CC: [email protected]
CC: [email protected]
Signed-off-by: Sergei Trofimovich <[email protected]>
---
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


2023-10-04 21:30:49

by Sergei Trofimovich

[permalink] [raw]
Subject: Re: [PATCH] uapi: increase MAX_ARG_STRLEN from 128K to 6M

On Sun, Sep 24, 2023 at 08:30:05PM +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`).
>
> 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.
>
> 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?
>
> CC: Eric Biederman <[email protected]>
> CC: Kees Cook <[email protected]>
> CC: [email protected]
> CC: [email protected]
> Signed-off-by: Sergei Trofimovich <[email protected]>

Ping.

Also +CC: Andrew Morton <[email protected]>
in case mm tree is a reasonable place for this change.

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

--

Sergei