From patchwork Wed Aug 29 13:24:15 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Liebler X-Patchwork-Id: 29100 Received: (qmail 32747 invoked by alias); 29 Aug 2018 13:25:07 -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 32543 invoked by uid 89); 29 Aug 2018 13:24:54 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_LOW, SEM_URI, SEM_URIRED, SPF_PASS, TIME_LIMIT_EXCEEDED autolearn=unavailable version=3.3.2 spammy=Furthermore X-HELO: mx0a-001b2d01.pphosted.com Subject: Re: [patch] Fix BZ 23400 -- stdlib/test-bz22786.c creates temporary files in glibc source tree To: libc-alpha@sourceware.org References: <910a25b4-8df2-8ac0-6859-1431d60b5265@linaro.org> From: Stefan Liebler Date: Wed, 29 Aug 2018 15:24:15 +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: 18082913-0016-0000-0000-000001FD5F48 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18082913-0017-0000-0000-00003253D61B Message-Id: <7a34578f-291f-886a-32ad-9542740c5043@linux.ibm.com> Hi Paul, If I run the test on s390 (31bit), the test fails with: malloc: unable to allocate 2147483675 bytes: Cannot allocate memory warning: could not remove temporary file: /tmp/bz22786.iaoLYS: Directory not empty This test tries to allocate a little bit too much bytes for 31bit. Thus the former implementation returned EXIT_UNSUPPORTED. Now the test is failing due to xmalloc. As the symlink was created within the temporary directory /tmp/bz22786.XXXXXX, but was not unlinked by the test, support_delete_temp_files() fails to remove it. Can we just return EXIT_UNSUPPORTED on 31bit as done with the attached patch? Bye Stefan On 08/06/2018 05:12 PM, Paul Pluzhnikov wrote: > Thanks for review! > > On Mon, Jul 30, 2018 at 1:13 PM Adhemerval Zanella > wrote: > >>> + strcpy (lnk, dir); >>> + strcat (lnk, "/symlink"); >> >> Maybe just 'char *lnk = xasprintf ("%s/symlink", dir);' instead? > > Done. > >>> + if (symlink (".", lnk) != 0) >>> { >>> printf ("symlink (%s, %s): %m\n", dir, lnk); >>> return EXIT_FAILURE; >> >> Use FAIL_EXIT1 or just TEST_VERIFY_EXIT. > > Done. > >>> memset (p, 'a', path_len - (path - p) - 2); >>> p[path_len - (path - p) - 1] = '\0'; >> >> Shouldn't it 'p - path' instead? The subtraction is clearly issuing a >> overflow and I think it is not what the test meant here. > > Good catch. Turns out that this was a buffer overflow in the original > test. Fixed. > > Thanks, > > 2018-08-06 Paul Pluzhnikov > > [BZ #23400] > * stdlib/test-bz22786.c (do_test): Fix undefined behavior. > Reviewed-by: Carlos O'Donell commit 5ada1975be8f1b30b8f33d1d25cb5575690066e1 Author: Stefan Liebler Date: Wed Aug 29 15:20:51 2018 +0200 Test stdlib/test-bz22786 exits now with unsupported if malloc fails. The test tries to allocate more than 2^31 bytes which will always fail on s390 as it has maximum 31bit of memory. Before commit 6c3a8a9d868a8deddf0d6dcc785b6d120de90523, this test returned unsupported if malloc fails. This patch re enables this behaviour. Furthermore support_delete_temp_files() failed to remove the temp directory in this case as it is not empty due to the created symlink. Thus the creation of the symlink is moved behind malloc. ChangeLog * stdlib/test-bz22786.c (do_test): Return EXIT_UNSUPPORTED if malloc fails. diff --git a/stdlib/test-bz22786.c b/stdlib/test-bz22786.c index d1aa69106c..44ec631a96 100644 --- a/stdlib/test-bz22786.c +++ b/stdlib/test-bz22786.c @@ -39,16 +39,21 @@ do_test (void) const char *lnk = xasprintf ("%s/symlink", dir); const size_t path_len = (size_t) INT_MAX + strlen (lnk) + 1; - TEST_VERIFY_EXIT (symlink (".", lnk) == 0); - DIAG_PUSH_NEEDS_COMMENT; #if __GNUC_PREREQ (7, 0) /* GCC 7 warns about too-large allocations; here we need such allocation to succeed for the test to work. */ DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); #endif - char *path = xmalloc (path_len); + char *path = malloc (path_len); DIAG_POP_NEEDS_COMMENT; + if (path == NULL) + { + printf ("malloc (%zu): %m\n", path_len); + return EXIT_UNSUPPORTED; + } + + TEST_VERIFY_EXIT (symlink (".", lnk) == 0); /* Construct very long path = "/tmp/bz22786.XXXX/symlink/aaaa....." */ char *p = mempcpy (path, lnk, strlen (lnk));