Re: [PATCH 02/12] x86-64: Add endbr64 to tst-quadmod[12].S

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

On Mon, Jul 23, 2018 at 7:53 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 07/21/2018 10:20 AM, H.J. Lu wrote:
>> Add endbr64 to tst-quadmod1.S and tst-quadmod2.S so that func and foo
>> can be called indirectly.
>>
>>       * sysdeps/x86_64/tst-quadmod1.S (func): Add endbr64 if IBT is
>>       enabled.
>>       (foo): Likewise.
>>       * sysdeps/x86_64/tst-quadmod2.S (func) : Likewise.
>>       (foo): Likewise.
>> ---
>>  sysdeps/x86_64/tst-quadmod1.S | 6 ++++++
>>  sysdeps/x86_64/tst-quadmod2.S | 6 ++++++
>>  2 files changed, 12 insertions(+)
>
> This is OK as-is, but marking foo with enbr64 seems
> beyond what is needed. Only foo calls func directly,

Both func and foo needs ENDBR64.   All global functions
may be called indirectly via PLT:

[hjl@gnu-cet-2 build-x86_64-linux]$ readelf -rW elf/tst-quad1

Relocation section '.rela.dyn' at offset 0x7f0 contains 2 entries:
    Offset             Info             Type               Symbol's
Value  Symbol's Name + Addend
0000000000403ff0  0000000100000006 R_X86_64_GLOB_DAT
0000000000000000 __libc_start_main@GLIBC_2.2.5 + 0
0000000000403ff8  0000000200000006 R_X86_64_GLOB_DAT
0000000000000000 __gmon_start__ + 0

Relocation section '.rela.plt' at offset 0x820 contains 1 entry:
    Offset             Info             Type               Symbol's
Value  Symbol's Name + Addend
0000000000404018  0000000300000007 R_X86_64_JUMP_SLOT
0000000000000000 foo + 0
[hjl@gnu-cet-2 build-x86_64-linux]$

Without ENDBR64, I got

(gdb) r --direct
Starting program:
/export/build/gnu/tools-build/glibc-cet/build-x86_64-linux/elf/tst-quad1
--direct

Program received signal SIGSEGV, Segmentation fault.
foo () at ../sysdeps/x86_64/tst-quadmod1.S:43
43 movq .Ljmp(%rip), %rax
(gdb)

I will check it as-is.

> so I would expect only func needing markup. If you can
> tighten this up it would be better.
>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
>>
>> diff --git a/sysdeps/x86_64/tst-quadmod1.S b/sysdeps/x86_64/tst-quadmod1.S
>> index 26f2f1b599..c60f9dc89d 100644
>> --- a/sysdeps/x86_64/tst-quadmod1.S
>> +++ b/sysdeps/x86_64/tst-quadmod1.S
>> @@ -28,6 +28,9 @@
>>       .type   func, @function
>>  func:
>>       .cfi_startproc
>> +#if defined __CET__ && (__CET__ & 1) != 0
>> +     endbr64
>> +#endif
>
> OK.
>
>>       xorl    %edi, %edi
>>       jmp     exit@PLT
>>       .cfi_endproc
>> @@ -37,6 +40,9 @@ func:
>>  foo:
>>       .cfi_startproc
>>       .cfi_def_cfa_register 6
>> +#if defined __CET__ && (__CET__ & 1) != 0
>> +     endbr64
>
> OK.
>
>> +#endif
>>       movq    .Ljmp(%rip), %rax
>>       subq    $BIAS, %rax
>>       jmp     *%rax
>> diff --git a/sysdeps/x86_64/tst-quadmod2.S b/sysdeps/x86_64/tst-quadmod2.S
>> index e923adf672..af03444d4f 100644
>> --- a/sysdeps/x86_64/tst-quadmod2.S
>> +++ b/sysdeps/x86_64/tst-quadmod2.S
>> @@ -27,6 +27,9 @@
>>       .type   func, @function
>>  func:
>>       .cfi_startproc
>> +#if defined __CET__ && (__CET__ & 1) != 0
>> +     endbr64
>
> OK. Foo calls func directly.
>
>> +#endif
>>       xorl    %edi, %edi
>>       jmp     exit@PLT
>>       .cfi_endproc
>> @@ -36,6 +39,9 @@ func:
>>  foo:
>>       .cfi_startproc
>>       .cfi_def_cfa_register 6
>> +#if defined __CET__ && (__CET__ & 1) != 0
>> +     endbr64
>> +#endif
>>       movq    .Ljmp(%rip), %rax
>>       subq    $BIAS, %rax
>>       jmp     *%rax
>>
>
> OK.
>
> c.



-- 
H.J.