Re: [PATCH] libio: Eliminate _IO_stdin, _IO_stdout, _IO_stderr

[Florian Weimer <fweimer at redhat dot com>]

* Dmitry V. Levin:

> On Mon, Feb 18, 2019 at 10:58:47AM +0100, Florian Weimer wrote:
>> * Dmitry V. Levin:
>> 
>> > Looks like _IO_legacy_file makes sense only when &_IO_stdin_used == NULL.
>> > If the check was moved inside _IO_legacy_file, then ...
>> 
>> I'm not sure.  We have seen cases where new binaries do not define
>> _IO_stdin_used, perhaps related to symbol visibility.
>> 
>> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=634261>
>> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=816059>
>> <https://bugs.launchpad.net/ubuntu/+source/lua5.3/+bug/1570055>
>
> Yes, and we also have
> https://sourceware.org/bugzilla/show_bug.cgi?id=17908
>
>> So at least for the free call (in _IO_deallocate_file after the patch),
>> I really want to check that the object isn't any of the preallocated
>> ones, in case the crashes aren't immediate and we have the potential
>> here for causing heap corruption.
>
> Fair enough.
>
> Why do we check
> 	(&_IO_stdin_used == NULL) && _IO_legacy_file (fp)
> instead of just
> 	_IO_legacy_file (fp)
> then?  Is it just an optimization?

Mainly to match the previous (buggy) implementation.

I think in _IO_fwide and _IO_old_file_init_internal, we could perform
the simplified check.  I've looked into that for my reply to Gabriel,
and the code assumes that we over-allocate even in the old
implementation, and only the objects that come from _IO_stdin_ etc. (the
three statically allocated FILE objects) are too small.

I don't think this assumption is entirely correct, and I'd prefer to
match the old behavior as close as possible here.  The reason for
eliminating _IO_stdin is a slight simplification of libio, not to fix
all those bugs.

Thanks,
Florian