This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Maintaining libio
On 19/04/2017 12:02, Florian Weimer wrote:
> 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.
This seems a more reasonable approach, I realised it was about _IO_str_seekoff
exposure right after send the email (sorry about that noise).