This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] libio: Eliminate _IO_stdin, _IO_stdout, _IO_stderr
* 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