Fix segfault in maybe_script_execute.
Commit Message
On 09/06/2018 10:50 AM, Andreas Schwab wrote:
> On Sep 06 2018, Stefan Liebler <stli@linux.ibm.com> wrote:
>
>> On 09/05/2018 03:44 PM, Andreas Schwab wrote:
>>> On Sep 05 2018, Stefan Liebler <stli@linux.ibm.com> wrote:
>>>
>>>> If argc > 1, the arguments inclusive the NULL termination of new_argv
>>>> is copied from argv with memcpy.
>>>
>>> But it has the same bug, doesn't it?
>>>
>>> Andreas.
>>>
>> Yes. It also has this bug.
>
> This makes the description rather confusing, implying there is a
> difference between argc == 1 and argc > 1, which isn't.
>
> Andreas.
>
Okay. I've adjusted the description.
Thanks.
Stefan
Comments
On Sep 06 2018, Stefan Liebler <stli@linux.ibm.com> wrote:
> commit 6359e1c59f856a1115fdbcfa233052ac62e32c83
> Author: Stefan Liebler <stli@linux.ibm.com>
> Date: Thu Sep 6 11:03:38 2018 +0200
>
> Fix segfault in maybe_script_execute.
>
> If glibc is built with gcc 8 and -march=z900,
> the testcase posix/tst-spawn4-compat crashes with a segfault.
>
> In function maybe_script_execute, the new_argv array is dynamically
> initialized on stack with (argc + 1) elements.
> The function wants to add _PATH_BSHELL as the first argument
> and writes out of bounds of new_argv.
>
> In case of argc == 1, it writes three instead of two elements:
> new_argv[0] = (char *) _PATH_BSHELL;
> new_argv[1] = (char *) args->file;
> new_argv[2] = NULL;
>
> The latter write access writes to the same location where the
> pointer args is stored on stack. Then it segfaults while accessing args:
> args->exec (new_argv[0], new_argv, args->envp);
>
> In case of argc > 1, new_argv[0] and new_argv[1] are set in the same
This still make a difference between argc == 1 and argc > 1. Just say
that there is an off-by-one because maybe_script_execute fails to count
the terminating NULL when sizing new_argv.
Andreas.
commit 6359e1c59f856a1115fdbcfa233052ac62e32c83
Author: Stefan Liebler <stli@linux.ibm.com>
Date: Thu Sep 6 11:03:38 2018 +0200
Fix segfault in maybe_script_execute.
If glibc is built with gcc 8 and -march=z900,
the testcase posix/tst-spawn4-compat crashes with a segfault.
In function maybe_script_execute, the new_argv array is dynamically
initialized on stack with (argc + 1) elements.
The function wants to add _PATH_BSHELL as the first argument
and writes out of bounds of new_argv.
In case of argc == 1, it writes three instead of two elements:
new_argv[0] = (char *) _PATH_BSHELL;
new_argv[1] = (char *) args->file;
new_argv[2] = NULL;
The latter write access writes to the same location where the
pointer args is stored on stack. Then it segfaults while accessing args:
args->exec (new_argv[0], new_argv, args->envp);
In case of argc > 1, new_argv[0] and new_argv[1] are set in the same
way as in the case above and the arguments inclusive the NULL termination
of new_argv is copied from argv with memcpy.
Note: The last copied element (NULL termination) is written out of
bounds of new_argv.
The implementation assumes that argc == 0 will never happen!
ChangeLog:
* sysdeps/unix/sysv/linux/spawni.c (maybe_script_execute):
Increment size of new_argv by one.
@@ -101,7 +101,7 @@ maybe_script_execute (struct posix_spawn_args *args)
ptrdiff_t argc = args->argc;
/* Construct an argument list for the shell. */
- char *new_argv[argc + 1];
+ char *new_argv[argc + 2];
new_argv[0] = (char *) _PATH_BSHELL;
new_argv[1] = (char *) args->file;
if (argc > 1)