Message ID | 2ed9096b-245f-9ddb-164d-46e137568146@linux.ibm.com |
---|---|
State | Committed |
Headers |
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: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> 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 <schwab@suse.de> Cc: GNU C Library <libc-alpha@sourceware.org>, Adhemerval Zanella <adhemerval.zanella@linaro.org> References: <6fab031c-9748-42e4-7137-55bf9ae59545@linux.ibm.com> <mvmd0tsf49k.fsf@suse.de> <032ef9d4-c359-01ca-7af1-d66730f1d050@linux.ibm.com> <mvmsh2ndn6t.fsf@suse.de> <f69c7e70-b0b0-b635-1582-0a3ad36de981@linux.ibm.com> <mvmo9dbdm33.fsf@suse.de> From: Stefan Liebler <stli@linux.ibm.com> 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: <mvmo9dbdm33.fsf@suse.de> Content-Type: multipart/mixed; boundary="------------D1BB13A4422212E984FA2204" 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> |
Commit Message
Stefan Liebler
Sept. 6, 2018, 10:29 a.m. UTC
On 09/06/2018 11:14 AM, Andreas Schwab wrote: > 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. > What about this? Stefan
Comments
On Sep 06 2018, Stefan Liebler <stli@linux.ibm.com> wrote: > commit 03a8c686eab8e12b4c478e7606963db6a72f6f0e > Author: Stefan Liebler <stli@linux.ibm.com> > 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. Ok, thanks. Andreas.
On 09/06/2018 12:33 PM, Andreas Schwab wrote: > On Sep 06 2018, Stefan Liebler <stli@linux.ibm.com> wrote: > >> commit 03a8c686eab8e12b4c478e7606963db6a72f6f0e >> Author: Stefan Liebler <stli@linux.ibm.com> >> 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. > > Ok, thanks. > > Andreas. > Committed. Thanks. Stefan
Thanks for the patch, could you check if we need to backport this fix to older releases as well? Inviato da iPhone > Il giorno 06 set 2018, alle ore 13:30, Stefan Liebler <stli@linux.ibm.com> ha scritto: > >> On 09/06/2018 12:33 PM, Andreas Schwab wrote: >>> On Sep 06 2018, Stefan Liebler <stli@linux.ibm.com> wrote: >>> commit 03a8c686eab8e12b4c478e7606963db6a72f6f0e >>> Author: Stefan Liebler <stli@linux.ibm.com> >>> 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. >> Ok, thanks. >> Andreas. > > Committed. > > Thanks. > Stefan >
On 09/07/2018 09:57 AM, Adhemerval Zanella wrote: > Thanks for the patch, could you check if we need to backport this fix to older releases as well? > As far as I've seen, maybe_script_execute was introduced with commit "posix: New Linux posix_spawn{p} implementation" (https://sourceware.org/git/?p=glibc.git;a=commit;h=9ff72da471a509a8c19791efe469f47fa6977410) => first release was glibc 2.24. Should it be backported to all versions: glibc 2.24 ... 2.28? Bye Stefan > Inviato da iPhone > >> Il giorno 06 set 2018, alle ore 13:30, Stefan Liebler <stli@linux.ibm.com> ha scritto: >> >>> On 09/06/2018 12:33 PM, Andreas Schwab wrote: >>>> On Sep 06 2018, Stefan Liebler <stli@linux.ibm.com> wrote: >>>> commit 03a8c686eab8e12b4c478e7606963db6a72f6f0e >>>> Author: Stefan Liebler <stli@linux.ibm.com> >>>> 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. >>> Ok, thanks. >>> Andreas. >> >> Committed. >> >> Thanks. >> Stefan >> >
> Il giorno 07 set 2018, alle ore 11:33, Stefan Liebler <stli@linux.ibm.com> ha scritto: > >> On 09/07/2018 09:57 AM, Adhemerval Zanella wrote: >> Thanks for the patch, could you check if we need to backport this fix to older releases as well? > As far as I've seen, maybe_script_execute was introduced with commit "posix: New Linux posix_spawn{p} implementation" (https://sourceware.org/git/?p=glibc.git;a=commit;h=9ff72da471a509a8c19791efe469f47fa6977410) > => first release was glibc 2.24. > > Should it be backported to all versions: glibc 2.24 ... 2.28? > Ideally yes, this is quite simple patch so I thin backport should not require extra work.
Hi, I've cherry picked the commit "Fix segfault in maybe_script_execute." (https://sourceware.org/git/?p=glibc.git;a=commit;h=28669f86f6780a18daca264f32d66b1428c9c6f1) and it is now available on glibc release branches 2.24 ... 2.28. Bye Stefan On 09/07/2018 03:36 PM, Adhemerval Zanella wrote: > > >> Il giorno 07 set 2018, alle ore 11:33, Stefan Liebler <stli@linux.ibm.com> ha scritto: >> >>> On 09/07/2018 09:57 AM, Adhemerval Zanella wrote: >>> Thanks for the patch, could you check if we need to backport this fix to older releases as well? >> As far as I've seen, maybe_script_execute was introduced with commit "posix: New Linux posix_spawn{p} implementation" (https://sourceware.org/git/?p=glibc.git;a=commit;h=9ff72da471a509a8c19791efe469f47fa6977410) >> => first release was glibc 2.24. >> >> Should it be backported to all versions: glibc 2.24 ... 2.28? >> > > Ideally yes, this is quite simple patch so I thin backport should not require extra work. >
Tyvm. Inviato da iPhone > Il giorno 10 set 2018, alle ore 13:32, Stefan Liebler <stli@linux.ibm.com> ha scritto: > > Hi, > > I've cherry picked the commit "Fix segfault in maybe_script_execute." (https://sourceware.org/git/?p=glibc.git;a=commit;h=28669f86f6780a18daca264f32d66b1428c9c6f1) and it is now available on glibc release branches 2.24 ... 2.28. > > Bye > Stefan > > On 09/07/2018 03:36 PM, Adhemerval Zanella wrote: >>> Il giorno 07 set 2018, alle ore 11:33, Stefan Liebler <stli@linux.ibm.com> ha scritto: >>> >>>> On 09/07/2018 09:57 AM, Adhemerval Zanella wrote: >>>> Thanks for the patch, could you check if we need to backport this fix to older releases as well? >>> As far as I've seen, maybe_script_execute was introduced with commit "posix: New Linux posix_spawn{p} implementation" (https://sourceware.org/git/?p=glibc.git;a=commit;h=9ff72da471a509a8c19791efe469f47fa6977410) >>> => first release was glibc 2.24. >>> >>> Should it be backported to all versions: glibc 2.24 ... 2.28? >>> >> Ideally yes, this is quite simple patch so I thin backport should not require extra work. >
commit 03a8c686eab8e12b4c478e7606963db6a72f6f0e Author: Stefan Liebler <stli@linux.ibm.com> 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)