From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/13228 Path: news.gmane.org!.POSTED!not-for-mail From: Markus Wichmann Newsgroups: gmane.linux.lib.musl.general Subject: Re: string-backed FILEs mess Date: Wed, 12 Sep 2018 17:09:41 +0200 Message-ID: <20180912150941.GB13976@voyager> References: <20180912140239.GV1878@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1536764883 22496 195.159.176.226 (12 Sep 2018 15:08:03 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 12 Sep 2018 15:08:03 +0000 (UTC) User-Agent: Mutt/1.10.1 (2018-07-13) To: musl@lists.openwall.com Original-X-From: musl-return-13244-gllmg-musl=m.gmane.org@lists.openwall.com Wed Sep 12 17:07:59 2018 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1g06kC-0005bU-DZ for gllmg-musl@m.gmane.org; Wed, 12 Sep 2018 17:07:52 +0200 Original-Received: (qmail 3401 invoked by uid 550); 12 Sep 2018 15:10:01 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 3383 invoked from network); 12 Sep 2018 15:10:01 -0000 Content-Disposition: inline In-Reply-To: <20180912140239.GV1878@brightrain.aerifal.cx> X-Provags-ID: V03:K1:fWr01Nr/G1tsyn/5B3i2arPmZi5W/U8F1lDcYkH+x0Q2h12PoRc TQGKuy00aHL45Htavk556i7NiZN/PkpBCsFsr3zW4SIanKW7trgZE/vCHlE9IiKmQTqI12i l90SkTh98hmLOS4r1Y2OdBdEynIy7jZW7pWbSZIBo7E1B8pZEuKaQtQtJPEHL7PR0Zvtqpx NWUmyj2BOIma8x4FIa5QA== X-UI-Out-Filterresults: notjunk:1;V01:K0:VkRykOjNEUs=:i4gp0p3CD6h4L9nFyRvcma XsgFZuxh5MKVhQ9XxCSEe5Z2BxTDSgBHrV7EWVr3+QtlIeAu5Fbw4nsMxu6b+OkBQ6UEMAuUu wYaGV9RmHy503dNppoZMljxxaHZE2UURZR41zaFk5CRYqe81UIuIdGoeFl77q+58HnHhHQSmv a84B/Q1PFNUxtZjWaCvuQ2MNJOc5poXzeqxkdUON3PrI6pgoJKYnTyVFRQnaCVl0QlEFr2JDZ OvoanGaeAPBZte+ixmfdQ08HaFvk4hm2QO7WjJlvTGjwKMX6gbfUguds4Rmt87Zh3I1vgijJ/ JJT4PyqauYGKG+Uo9Ft1oktJCBoFpSRKcXVvQiuTVZQXhHl7YRU1o29/TviHkJ5ZNvBVr/unA 7vGYZf07Iufb/O5aDquUgxIbIQCUga1jiOIa2S2Z7VdsGvAC/Y3YNP/fVFqvWbmuL2bSPOPvS jfV/ypfX8uGtqdA7NLhZX8Zj22fTw2pMKNQZytz97EUHmv+9BbsPLIIZl6BCW/MZ9af7BPlpT 9ARBHEedFl4sQJOixUHDRGnJqtLj0zOwIRlvaBf1bRclE75L2vAHnqwtWXg7NbTMmYKC5xga2 lplB22eFoc0kw82FwBNfhG5KZFb3SzCrxI5q9jLuFlbVUikLsGrTz2kl/TDmRxabBWTKxPJA8 n2+5KKIUSli/3KejcgjXVvZzvm9Dy/vhs3vHkxztKE9vJLVJf9XVQInYTqNP08k0g4796p3di Y7RUl/h9Ldwp5AsR8urMv0e8frTwxJcyc/mxQzzhHidOu5AeBXMAiwfX/WtZ34JFC2bzOxsz Xref: news.gmane.org gmane.linux.lib.musl.general:13228 Archived-At: On Wed, Sep 12, 2018 at 10:02:39AM -0400, Rich Felker wrote: > While working on the headers/declarations/hidden stuff, I've run > across some issues that should be documented as needing fix. One big > one is this: > > strto* is still setting up a fake FILE with buffer end pointer as > (void*)-1 as a hack to read directly from a string of unknown length. > This is of course nonsensical UB and might lead to actual problems > (signed overflow) in places where rend-rpos is computed. strtol.c > handles that aspect already by "if ((size_t)s > (size_t)-1/2)" but > it's still horribly wrong. There's a __string_read function sscanf > uses that avoids the whole problem by incrementally measuring string > length and copying it to a real stdio buffer, which also makes ungetc > safe, and this is the obvious *right* thing to do, but it might make > strto* unacceptably slower. I haven't done any measurement. > Well, first of all, I might set my foot wrong here very badly, but I generally don't care about C standard UB as long as the behavior is defined elsewhere. In this case, the behavior is undefined because the C standard only defines a partial mapping from pointer to integer and back (namely null pointer <-> 0 and non-null pointer <-> nonzero, but nothing else), and that is why most operations on pointers are undefined. However, Linux defines that it only works on a linear address space, and so completes the mapping from pointer to integer, and so avoids the UB by defining it. We have done similar things in the past, like assuming long to be large enough for a pointer, as Linux depends on that as well. As for the overflow: That would be fixed if you set rend to rpos + PTRDIFF_MAX unless that overflows. Technically still UB, but at least the calculations work now. The __string_read approach shouldn't be all that much slower, though. It only adds overhead when the read buffer is empty, which is going to be on the first call and then every 256 characters. And the overhead is mostly a memchr()... on second thought, that might take a while. > The other "mostly right" (modulo ungetc not being available then) > approach would be getting rid of the whole current buffer design with > start/end pointers and using indices instead. This would solve a lot > of stupid gratuitous UB in stdio, like (T*)0-(T*)0 being undefined. > It's not clear to me whether it would be more or less efficient. It > would "break" glibc ABI-compat for stdio -- the original reason I used > the pointer-based design -- but that could be fixed by putting > "must-be-null" fields in place of the buffer pointers so that any > glibc code using getc_unlocked/putc_unlocked macros would hit the > "no buffer space" code path and call an actual function. In many ways > that's desirable anyway. > I don't know how important the glibc-compat still is for people. I don't need it, but then, I have glibc available if necessary. And with Steam moving to Linux, closed-source software is certainly going to be on the rise for Linux/PC (i.e. Linux/i386 and Linux/amd64). Which is going to be a problem for musl-based distributions, if they want to support that. > Probably the right next step here is measuring whether just using > __string_read would make anything measurably slower. > > Rich Probably. Although the memchr() in that function might access past the string, but only if the string isn't terminated, so we can invoke the GIGO principle, right? Ciao, Markus