Maintaining libio

[Florian Weimer]

On 04/19/2017 04:48 PM, Adhemerval Zanella wrote:
> 
> 
> On 19/04/2017 06:17, Florian Weimer wrote:
>> On 08/08/2016 10:43 PM, Adhemerval Zanella wrote:
>>> @@ -239,14 +254,14 @@ _IO_str_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>>>      if (mode == 0 && (fp->_flags & _IO_TIED_PUT_GET))
>>>        mode = (fp->_flags & _IO_CURRENTLY_PUTTING ? _IOS_OUTPUT : _IOS_INPUT);
>>>    +  bool was_writing = (fp->_IO_write_ptr > fp->_IO_write_base
>>> +             || _IO_in_put_mode (fp));
>>> +  if (was_writing)
>>> +    _IO_str_switch_to_get_mode (fp);
>>
>> This patch breaks backwards compatibility with applications which call _IO_str_seekoff directly.  This is an exported function and it was originally intended as a building block for building custom streams, so we cannot change what it does just to fit glibc's internal needs, based on how the function is called from within glibc.
>>
>> But if we apply this standard of backwards compatibility, we cannot make *any* changes to libio (including important security hardening) without copying most of the code first.  We have no tests which check the extended API behavior, and the interface is very much under-documented, too.
>>
>> What should we do here?
> 
> Right, so should we revert the patch, reopen both bugs and rework all libio
> in this case (which might span on multiple releases)? I know that we should
> aim for compatibility where applicable, but I think blindly aim for it even
> for bug/out of conformance cases adds more maintainer burden that actually
> fixes real cases usage.
> 
> For this specific case, the code is clearly buggy when ran a different libc
> for a non-specific gnu extension (open_memstream). Should we still provide
> buggy compat interfaces in this case (as we are still aiming to provide)?

Sorry, I wasn't clear.  The buggy interface is open_memstream.  Fixing 
that is completely fine, no compatibility symbol is required.  But 
_IO_str_seekoff is a completely different, allegedly public interface, 
and the existing callers most expect some concrete behavior from it.

A hypothetical example of the same scenario: posix_fallocate used to 
have a bug that it did not work on O_APPEND descriptors because pwrite64 
ignored the offset for them, as required by POSIX.  We could have fixed 
that by changing pwrite64 not to ignore the write offset, but that's of 
course bogus because the specified semantics for pwrite64 require 
different behavior.

What I'm trying to say is that similar, but undocumented requirements 
might well exist for the _IO_str_seekoff function.  Hence my original 
comment that we'd need to make a copy first, fix the copy, and adjust 
internal callers to use the copy, leaving the original implementation alone.

Things get worse once we start changing struct definitions.  Then we 
basically have to duplicate everything that depends on those structs.

I think that's not a good use of our time because it is very unlikely 
that there are any applications left which use these interfaces. 
Instead, I suggest that we make explicit that the internal libio 
interfaces are unsupported (because they are unsupportable), and remove 
them from the ABI, so that people who have those old binaries get clear 
dynamic linker failures instead of corrupted data.

Thanks,
Florian