linux-kernel - Re: [PATCH] execve.2: execve also returns E2BIG if a string is too long

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZSa0ZotvTRCe88OQ@debian>
Date:   Wed, 11 Oct 2023 16:42:46 +0200
From:   Alejandro Colomar <alx@...nel.org>
To:     Rik van Riel <riel@...riel.com>
Cc:     linux-man@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
        kernel-team@...a.com, Eric Biederman <ebiederm@...ssion.com>
Subject: Re: [PATCH] execve.2: execve also returns E2BIG if a string is too
 long

Hi Rik,

On Wed, Oct 11, 2023 at 09:21:28AM -0400, Rik van Riel wrote:
> On Wed, 2023-10-11 at 12:41 +0200, Alejandro Colomar wrote:
> > Hi Rik,
> > 
> > On Tue, Oct 10, 2023 at 11:41:53PM -0400, Rik van Riel wrote:
> > > Document that if a command line or environment string is too long
> > > (> MAX_ARG_STRLEN), execve will also return E2BIG.
> > 
> > That's already implied by the current text:
> > 
> >        E2BIG  The total number of bytes in the environment (envp) and
> > argument
> >               list (argv) is too large.
> > 
> > That means that
> > 
> > size_t  bytes;
> > 
> > bytes = 0;
> > for (char *e = envp; e != NULL; e++)
> >         bytes += strlen(e) + 1;  // I have doubts about the +1
> > for (char *a = argv; a != NULL; a++)
> >         bytes += strlen(a) + 1;  // Same doubts
> > 
> > if (bytes > MAX_ARG_STRLEN)  // Maybe >= ?
> >         return -E2BIG;
> 
> The code in fs/exec.c enforces MAX_ARG_STRLEN against
> each individual string, not against the total.
> 
> If any string, either argument or environment, is larger
> than 32 * PAGE_SIZE, the kernel will return -E2BIG.
> 
> do_execveat_common() has this code, which uses copy_strings
> to copy both the strings from the environment, and from
> the command line arguments:
> 
>         retval = copy_strings(bprm->envc, envp, bprm);
>         if (retval < 0)
>                 goto out_free;
> 
>         retval = copy_strings(bprm->argc, argv, bprm);
>         if (retval < 0)
>                 goto out_free;
> 
> Inside copy_strings() we have this code:
> 
> 
>         while (argc-- > 0) {
> ...
>                 len = strnlen_user(str, MAX_ARG_STRLEN);
>                 if (!len)
>                         goto out;
> 
>                 ret = -E2BIG;
>                 if (!valid_arg_len(bprm, len))
>                         goto out;
> 
> The valid_arg_len() function does not need explanation:
> 
> static bool valid_arg_len(struct linux_binprm *bprm, long len)
> {
>         return len <= MAX_ARG_STRLEN;
> }
> 
> 
> The current man page wording is very clear about the total
> length being enforced, but IMHO not as clear about the limit
> that gets enforced on each individual string.
> 
> The total length limit of environment & commandline arguments
> is enforced by bprm_stack_limits(), and is checked against
> either 1/4 of the maximum stack size, or 3/4 of _STK_LIM, whichever
> is smaller. The MAX_ARG_STRLEN value does not come into play when
> enforcing the total.

Ahh, so the limit for each string is different than the limit for the
total.  That makes sense.  Sorry for the noise.

Cheers,
Alex

> 
> -- 
> All Rights Reversed.

-- 
<https://www.alejandro-colomar.es/>

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)