Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 13 Apr 2022 18:46:43 -0400
From: Rich Felker <dalias@...c.org>
To: libc-coord@...ts.openwall.com
Subject: Re: stdio_ext.h extensions for gnulib

On Wed, Apr 13, 2022 at 10:44:49AM +0200, Florian Weimer wrote:
> * Rich Felker:
> 
> > Early on in musl's history, we added a set of further extensions in
> > stdio_ext.h:
> >
> > size_t __freadahead(FILE *);
> > const char *__freadptr(FILE *, size_t *);
> > void __freadptrinc(FILE *, size_t);
> > void __fseterr(FILE *);
> >
> > The purpose of these functions was to provide a way for gnulib to do
> > the things they already insisted on doing, but without having access
> > to the FILE representation internals (which is how they implemented
> > these things for every other system at the time).
> 
> I assume the first three functions provide access to the read buffer,
> and do not perform locking or error checking (that is, incrementing past
> the end of the read-ahead value is undefined).
> 
> fseterr is the converse of clearerr, and therefore uses locking, right?

The ones we added back in 2012 don't do any significant checking or
locking, probably because the only user was gnulib and gnulib didn't
seem to care about getting that right in their own versions either.
But I think if we're coordinating this we should try to get it right.

__freadahead reports the amount of buffered data and, although it
would be subject to TOCTOU issues if you use the result on a FILE that
could be accessed from multiple threads, that's not really unlike
other functions thar report status subject to TOCTOU. My understanding
(I may be wrong) is that it's intended to be usable without direct
buffer access, e.g. just calling fgetc or fread using the result. So
it should probably honor normal locking semantics.

__freadptr and __freadptrinc do direct access that almost certainly
does not make sense without either holding a lock already or being the
sole user of the FILE, so I would lean towards not having them imply
any locking. I really dislike these functions to begin with too. Ours
currently lack any error checking; that should almost surely be fixed
(by setting error status on the stream if you try to inc by an invalid
amount, I guess, since there is no return type to indicate an error).

__fseterr should almost certainly use locking.

> > The topic recently came up again via Toybox, where the author is not
> > using gnulib but was looking for some of the same functionality. I'm
> > told Bionic is on-board with adding these, and it might make sense to
> > coordinate this with glibc and other libc implementations/other
> > systems, or even propose these as some sort of standard.
> 
> For glibc, the read buffer pointers are part of the ABI (even when
> ignoring the attempt to share the stream implementation with historic
> libstdc++ versions).  So we would probably provide inline definitions
> (unless they require locking).  But adding such functions seems fine to
> me.

That seems like a reasonable implementation choice.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.