From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 99979 invoked by alias); 25 Feb 2016 07:28:07 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 99965 invoked by uid 89); 25 Feb 2016 07:28:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 spammy=badly, Pop, Push, 30000000 X-HELO: zimbra.cs.ucla.edu Subject: Re: [PATCH 1/3] posix: Remove dynamic memory allocation from execl{e,p} To: Adhemerval Zanella , libc-alpha@sourceware.org References: <1456146172-12850-1-git-send-email-adhemerval.zanella@linaro.org> <1456146172-12850-2-git-send-email-adhemerval.zanella@linaro.org> From: Paul Eggert Message-ID: <56CEACFF.7010805@cs.ucla.edu> Date: Thu, 25 Feb 2016 08:58:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1456146172-12850-2-git-send-email-adhemerval.zanella@linaro.org> Content-Type: multipart/mixed; boundary="------------000808060506090509040104" X-SW-Source: 2016-02/txt/msg00791.txt.bz2 This is a multi-part message in MIME format. --------------000808060506090509040104 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-length: 812 Adhemerval Zanella wrote: > + int argc; > + va_list ap; > + va_start (ap, arg); > + for (argc = 1; va_arg (ap, const char *); argc++) > + continue; With my "no arbitrary limits" hat on, I noticed that this has undefined behavior if more than INT_MAX arguments are passed to execl. The existing code is no saint in this area (it messes up badly if more than UINT_MAX args are passed), but the new code should not make things worse, and we might as well fix the UINT_MAX bug while we're at it. Attached please find a contrived test case illustrating the bug on x86-64. This test succeeds on x86-64 now (in that the program prints "execl: Cannot allocate memory" and exits with status 0) but could crash with the proposed patch. Perhaps you can add this to the glibc test cases while you're at it. --------------000808060506090509040104 Content-Type: text/x-csrc; name="big.c" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="big.c" Content-length: 758 /* This program assumes x86-64. Use 'ulimit -s 30000000' before running. */ #include #include int main (void) { /* Number of extra arguments to pass to execl, counting the trailing NULL. Must be a multiple of 2. */ long n = 2147483648; char x[] = ""; /* Push the the trailing NULL onto the stack. */ asm ("pushq $0"); /* Push the the trailing arguments onto the stack. */ for (long i = 1; i < n; i++) asm ("pushq %0" :: "r" (x)); /* Call execl, without a trailing NULL. GCC may warn about this. */ execl ("/bin/true", "true", x, x, x, x, x, x, x, x); /* Pop the added arguments off the stack. */ asm ("addq %0, %%rsp" :: "r" (n * sizeof (char *))); perror ("execl"); _exit (0); } --------------000808060506090509040104--