From patchwork Thu Sep 6 10:29:32 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Liebler X-Patchwork-Id: 29215 Received: (qmail 98752 invoked by alias); 6 Sep 2018 10:29:43 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 98741 invoked by uid 89); 6 Sep 2018 10:29:42 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx0a-001b2d01.pphosted.com Subject: Re: [PATCH] Fix segfault in maybe_script_execute. To: Andreas Schwab Cc: GNU C Library , Adhemerval Zanella References: <6fab031c-9748-42e4-7137-55bf9ae59545@linux.ibm.com> <032ef9d4-c359-01ca-7af1-d66730f1d050@linux.ibm.com> From: Stefan Liebler Date: Thu, 6 Sep 2018 12:29:32 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: x-cbid: 18090610-0028-0000-0000-000002F529F6 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18090610-0029-0000-0000-000023AEAF29 Message-Id: <2ed9096b-245f-9ddb-164d-46e137568146@linux.ibm.com> On 09/06/2018 11:14 AM, Andreas Schwab wrote: > On Sep 06 2018, Stefan Liebler wrote: > >> commit 6359e1c59f856a1115fdbcfa233052ac62e32c83 >> Author: Stefan Liebler >> 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. > What about this? Stefan commit 03a8c686eab8e12b4c478e7606963db6a72f6f0e Author: Stefan Liebler Date: Thu Sep 6 12:27: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. There is an off-by-one because maybe_script_execute fails to count the terminating NULL when sizing new_argv. ChangeLog: * sysdeps/unix/sysv/linux/spawni.c (maybe_script_execute): Increment size of new_argv by one. diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c index cf0213ece5..85239cedbf 100644 --- a/sysdeps/unix/sysv/linux/spawni.c +++ b/sysdeps/unix/sysv/linux/spawni.c @@ -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)