Re: [PATCH 1/2] Add private_function for private functions within glibc

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Sat, Jun 17, 2017 at 6:42 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 06/17/2017 03:06 PM, H.J. Lu wrote:
>> since shadow stack doesn't match return stack.  We need to use register
>> indirect branch via %ecx.  That means only 2 parameters can be passed
>> in registers for external function calls with lazy binding.  However,
>> internal_function, which should be used only with hidden function, is
>> defined as
>>
>>  # define internal_function __attribute__ ((regparm (3), stdcall))
>>
>> and used with private function calls between different shared objects of
>> glibc.  We introduce private_function for such purpose:
>>
>>  # define private_function __attribute__ ((regparm (2), stdcall))
>>
>> so that %ecx can be used by _dl_runtime_resolve as scratch register.
>>
>> Any comments?
>
> I have previously suggested to get rid of internal_function.  For
> example, applying it to static functions is generally unnecessary, but
> we still do that a lot.  (I know the static linkage case isn't what you
> are after here.)

I tried  and it didn't work since some i386 assembly codes call
internal functions directly:

_dl_start_user:\n\
        # Save the user entry point address in %edi.\n\
        movl %eax, %edi\n\
        # Point %ebx at the GOT.\n\
        call 0b\n\
        addl $_GLOBAL_OFFSET_TABLE_, %ebx\n\
        # See if we were run as a command with the executable file\n\
        # name as an extra leading argument.\n\
        movl _dl_skip_args@GOTOFF(%ebx), %eax\n\
        # Pop the original argument count.\n\
        popl %edx\n\
        # Adjust the stack pointer to skip _dl_skip_args words.\n\
        leal (%esp,%eax,4), %esp\n\
        # Subtract _dl_skip_args from argc.\n\
        subl %eax, %edx\n\
        # Push argc back on the stack.\n\
        push %edx\n\
        # The special initializer gets called with the stack just\n\
        # as the application's entry point will see it; it can\n\
        # switch stacks if it moves these contents over.\n\
" RTLD_START_SPECIAL_INIT "\n\
        # Load the parameters again.\n\
        # (eax, edx, ecx, *--esp) = (_dl_loaded, argc, argv, envp)\n\
        movl _rtld_local@GOTOFF(%ebx), %eax\n\
        leal 8(%esp,%edx,4), %esi\n\
        leal 4(%esp), %ecx\n\
        movl %esp, %ebp\n\
        # Make sure _dl_init is run with 16 byte aligned stack.\n\
        andl $-16, %esp\n\
        pushl %eax\n\
        pushl %eax\n\
        pushl %ebp\n\
        pushl %esi\n\
        # Clear %ebp, so that even constructors have terminated backchain.\n\
        xorl %ebp, %ebp\n\
        # Call the function to run the initializers.\n\
        call _dl_init\n\
        # Pass our finalizer function to the user in %edx, as per ELF ABI.\n\
        leal _dl_fini@GOTOFF(%ebx), %edx\n\
        # Restore %esp _start expects.\n\
        movl (%esp), %esp\n\
        # Jump to the user's entry point.\n\
        jmp *%edi\n\
        .previous\n\

Here parameters are passed to _dl_init in registers.  I want to minimize
changes to avoid any potential issues.

> I wonder what the change in code size due to a change from 3 to 2 to 0
> register parameters is.
>
> But even if we fix glibc's own use of register parameters, we risk
> running into bugs like bug 21265.  I'm not sure if this is worth the
> trouble at this point, especially since i386 has to be considered a
> legacy architecture at this point.
>
> Thanks,
> Florian



-- 
H.J.