From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from zimbra.cs.ucla.edu (zimbra.cs.ucla.edu [131.179.128.68]) by sourceware.org (Postfix) with ESMTPS id 032253858D35 for ; Sun, 16 Jan 2022 02:25:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 032253858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=cs.ucla.edu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=cs.ucla.edu Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 3A02A16019F; Sat, 15 Jan 2022 18:25:31 -0800 (PST) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id 5QtPJRJsxd1P; Sat, 15 Jan 2022 18:25:29 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 0DB741601A0; Sat, 15 Jan 2022 18:25:29 -0800 (PST) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id jDcZnL06NPKV; Sat, 15 Jan 2022 18:25:28 -0800 (PST) Received: from [192.168.1.9] (cpe-172-91-119-151.socal.res.rr.com [172.91.119.151]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id D388016019F; Sat, 15 Jan 2022 18:25:28 -0800 (PST) Message-ID: Date: Sat, 15 Jan 2022 18:25:27 -0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.1 Content-Language: en-US To: Martin Sebor References: From: Paul Eggert Organization: UCLA Computer Science Department Cc: libc-alpha@sourceware.org Subject: Re: [PATCH] avoid -Wuse-after-free [BZ #26779] In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 16 Jan 2022 02:25:33 -0000 On 1/15/22 16:21, Martin Sebor via Libc-alpha wrote: > + intptr_t ip_new_pool = (intptr_t)new_pool; > + intptr_t ptr_diff = ip_new_pool - ip_string_space; This sort of thing can have undefined behavior due to signed integer overflow. To avoid that problem on glibc platforms, you can use uintptr_t instead of intptr_t. > - map[i].alias += new_pool - string_space; > - map[i].value += new_pool - string_space; > + map[i].alias += ptr_diff; > + map[i].value += ptr_diff; Although this might pacify GCC 12, it continues to have the undefined behavior that GCC is evidently warning about, since the old values of map[i].alias and map[i].value have been freed and this means programs cannot use those pointers even if only to add something to them. It's conceivable that future GCC versions will figure this out and generate code that doesn't do what we want here. One simple workaround would be for the .alias and .value components to be offsets into the storage pool, instead of being absolute pointers. > --- a/stdlib/setenv.c > +++ b/stdlib/setenv.c > @@ -150,7 +150,9 @@ __add_to_environ (const char *name, const char *value, const char *combined, > { > char **new_environ; > > - /* We allocated this space; we can extend it. */ > + /* We allocated this space; we can extend it. Avoid using the raw > + reallocated pointer to avoid GCC -Wuse-after-free. */ > + uintptr_t ip_last_environ = (uintptr_t)last_environ; > new_environ = (char **) realloc (last_environ, > (size + 2) * sizeof (char *)); > if (new_environ == NULL) Cleaner would be to leave the old code alone, except to add this before reallocating: bool we_allocated_environ = __environ == last_environ; > ... > - if (__environ != last_environ) > + if ((uintptr_t)__environ != ip_last_environ) And then change the above line to "if (! we_allocated_environ)". > memcpy ((char *) new_environ, (char *) __environ, > size * sizeof (char *)); >