Re: [PATCH v3 2/4] libio: Remove the usage of __libc_IO_vtables

[Carlos O'Donell <carlos@redhat.com>]

On 3/6/23 08:43, Adhemerval Zanella Netto wrote:
> 
> 
> On 04/03/23 14:37, Carlos O'Donell wrote:
>> On 12/27/22 16:11, Adhemerval Zanella via Libc-alpha wrote:
>>> Instead of using a special ELF section along with a linker script
>>> directive to put the IO vtables within the RELRO section, the libio
>>> vtables are iall moved to an array marked as data.relro (so linker
>>
>> s/iall/all/g
>>
>>> will place in the RELRO segment without the need of extra directives).
>>>
>>> To avoid static linking namespace issues and to pulling all vtables
>>
>> s/to pulling/including/g
>> s/vtables/vtable/g
>>
>>> referenced objects, all required function pointers are set to weak alias.
>>>
>>
>> Have you tested this against legacy applications that interpose the vtables?
>>
>> In the original design Florian wrote:
>> ~~~
>>     To enable backwards compatibility, a special flag variable
>>     (_IO_accept_foreign_vtables), protected by the pointer guard, avoids
>>     process termination if libio stream object constructor functions have
>>     been called earlier.  Such constructor functions are called by the GCC
>>     2.95 libstdc++ library, and this mechanism ensures compatibility with
>>     old binaries.  Existing callers inside glibc of these functions are
>>     adjusted to call the original functions, not the wrappers which enable
>>     vtable compatiblity.
>>     
>>     The compatibility mechanism is used to enable passing FILE * objects
>>     across a static dlopen boundary, too.
>> ~~~
> 
> I have not, but we do have tests for these (which was added after the
> vtable hardening with 29055464a03c72762969a2e8734d0d05d4d70e58).  My
> understanding is it should cover the required interface for old binaries.

Agreed, that should cover the testing requirements, thanks for reviewing the existing
tests and seeing which apply to my request. That's something I should have done in more
detail as reviewer. Thank you for that.

It also meant I re-read the whole thread on "adding compat symbols requires testing old
behaviour" which was an interesting consensus discussion. I think we have achieved that
with Florian's new tests that were part of the bug 23313 fixes.

-- 
Cheers,
Carlos.