Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segfault in FallthroughFunctionPass #10

Closed
etherealvisage opened this issue Mar 2, 2018 · 12 comments
Closed

Segfault in FallthroughFunctionPass #10

etherealvisage opened this issue Mar 2, 2018 · 12 comments
Labels

Comments

@etherealvisage
Copy link
Contributor

Di (@SleepyMug) ran into a problem with parsing his libffi.so.6 (attached for reference), where it segfaults attempting to find function boundaries. The following lines from the debug output of etobjdump may be relevant:

Splitting code section into 29 fuzzy functions
disassembly error? fuzzyfunc-0x5fec 85 < 16e
WARNING: FallThrough: the last block was all NOPs
WARNING: FallThrough: the last block was all NOPs
etobjdump: pass/fallthrough.cpp:131: virtual void FallThroughFunctionPass::visit(Function*): Assertion `target' failed.
@etherealvisage
Copy link
Contributor Author

libffi.so.6.gz

@HidenoriKobayashi
Copy link
Contributor

HidenoriKobayashi commented Mar 2, 2018 via email

@SleepyMug
Copy link

SleepyMug commented Mar 2, 2018

I think it does, I'm on branch merging

$ app/etshell 
Welcome to the egalito shell version a5a63c8. Type "help" for usage.
egalito> parse2 /home/tenut/tmp/hey
creating ElfMap for file [/home/tenut/tmp/hey]

=== BUILDING ELF DATA STRUCTURES for [(executable)] ===
Number of version symbols: 12
Number of versions: 3
building relocation list
examining dependencies for ELF file
    depends on shared library [libffi.so.6]
    depends on shared library [libc.so.6]
    library at [/usr/lib/x86_64-linux-gnu/libffi.so.6]
    process [/usr/lib/x86_64-linux-gnu/libffi.so.6] a.k.a. libffi.so.6
    added new library [libffi.so.6]
    library at [/lib/x86_64-linux-gnu/libc.so.6]
    process [/lib/x86_64-linux-gnu/libc.so.6] a.k.a. libc.so.6
    added new library [libc.so.6]
--- RUNNING DEFAULT ELF PASSES for [(executable)] ---
Creating module from symbol info
WARNING: FallThrough: the last block was all NOPs
WARNING: FallThrough: the last block was all NOPs
WARNING: FallThrough: the last block was all NOPs
TIMING: 0.000057s for "FallThroughFunctionPass()"
Found data region at 0x0 size 0x97c
Found data region at 0x200df8 size 0x240
TIMING: 0.000009s for "HandleRelocsStrong(elf, relocList)"
TIMING: 0.000085s for "InternalCalls()"
TIMING: 0.000054s for "ExternalCalls(module->getPLTList())"
TIMING: 0.001051s for "SplitBasicBlock()"
TIMING: 0.000063s for "NonReturnFunction()"
TIMING: 0.000226s for "JumpTablePass()"
TIMING: 0.000008s for "JumpTableBounds()"
TIMING: 0.000005s for "JumpTableOverestimate()"
TIMING: 0.000115s for "SplitBasicBlock()"
TIMING: 0.000041s for "NonReturnFunction()"
TIMING: 0.000128s for "InferLinksPass(elf)"
found entry function [_start]
creating ElfMap for file [/usr/lib/x86_64-linux-gnu/libffi.so.6]

=== BUILDING ELF DATA STRUCTURES for [libffi.so.6] ===
creating ElfMap for file [/usr/lib/debug/.build-id/aa/1401f42d517693444b96c5774a62d4e8c84a35.debug]
Number of version symbols: 78
Number of versions: 6
building relocation list
examining dependencies for ELF file
    depends on shared library [libc.so.6]
    library at [/lib/x86_64-linux-gnu/libc.so.6]
--- RUNNING DEFAULT ELF PASSES for [libffi.so.6] ---
Creating module from symbol info
disassembly error? ffi_call_unix64 133 < 366
WARNING: FallThrough: the last block was all NOPs
WARNING: FallThrough: the last block was all NOPs
WARNING: FallThrough: the last block was all NOPs
etshell: pass/fallthrough.cpp:131: virtual void FallThroughFunctionPass::visit(Function*): Assertion `target' failed.
Aborted```

@HidenoriKobayashi
Copy link
Contributor

HidenoriKobayashi commented Mar 3, 2018 via email

@dwks
Copy link
Contributor

dwks commented Mar 3, 2018 via email

@SleepyMug
Copy link

It is, quoting comments from vpk, another good reason to consider xed as x86_64 disassembler. (which disassemble AAS correctly)

@dwks
Copy link
Contributor

dwks commented Mar 6, 2018

I used xed, but it doesn't understand this either:

$ ./obj/examples/xed -ir /tmp/ffi_call_unix64.bin -A -64 -mpx -cet
XDIS 6e: UNCOND_BR BASE       41FFE2                   jmp %r10
XDIS 71: BINARY    BASE       3D00000065               cmp $0x65000000, %eax
XDIS 76: BINARY    BASE       0000                     addb  %al, (%rax)
XDIS 78: BINARY    BASE       006F00                   addb  %ch, (%rdi)
XDIS 7b: BINARY    BASE       0000                     addb  %al, (%rax)
XDIS 7d: COND_BR   BASE       7500                     jnz 0x7f
XDIS 7f: BINARY    BASE       0000                     addb  %al, (%rax)
XDIS 81: COND_BR   BASE       7A00                     jp 0x83
XDIS 83: BINARY    BASE       0000                     addb  %al, (%rax)
ERROR: GENERAL_ERROR Could not decode at offset: 0x85 PC: 0x85: [3F]

There's a good reason for this, it's not actually valid instructions. From libffi source (unix64.S), we can see that this is in fact a manually specified inline jump table.

 86     /* The first byte of the flags contains the FFI_TYPE.  */
 87     movzbl  %cl, %r10d
 88     leaq    .Lstore_table(%rip), %r11
 89     movslq  (%r11, %r10, 4), %r10
 90     addq    %r11, %r10
 91     jmp *%r10
 92 
 93 .Lstore_table:
 94     .long   .Lst_void-.Lstore_table     /* FFI_TYPE_VOID */
 95     .long   .Lst_sint32-.Lstore_table   /* FFI_TYPE_INT */
 96     .long   .Lst_float-.Lstore_table    /* FFI_TYPE_FLOAT */
 97     .long   .Lst_double-.Lstore_table   /* FFI_TYPE_DOUBLE */
 98     .long   .Lst_ldouble-.Lstore_table  /* FFI_TYPE_LONGDOUBLE */
 99     .long   .Lst_uint8-.Lstore_table    /* FFI_TYPE_UINT8 */
100     .long   .Lst_sint8-.Lstore_table    /* FFI_TYPE_SINT8 */
101     .long   .Lst_uint16-.Lstore_table   /* FFI_TYPE_UINT16 */
102     .long   .Lst_sint16-.Lstore_table   /* FFI_TYPE_SINT16 */
103     .long   .Lst_uint32-.Lstore_table   /* FFI_TYPE_UINT32 */
104     .long   .Lst_sint32-.Lstore_table   /* FFI_TYPE_SINT32 */
105     .long   .Lst_int64-.Lstore_table    /* FFI_TYPE_UINT64 */
106     .long   .Lst_int64-.Lstore_table    /* FFI_TYPE_SINT64 */
107     .long   .Lst_struct-.Lstore_table   /* FFI_TYPE_STRUCT */
108     .long   .Lst_int64-.Lstore_table    /* FFI_TYPE_POINTER */
109 
110     .align 2
111 .Lst_void:
112     ret
113     .align 2
114 
115 .Lst_uint8:
116     movzbq  %al, %rax
117     movq    %rax, (%rdi)
118     ret
119     .align 2
120 .Lst_sint8:
121     movsbq  %al, %rax
122     movq    %rax, (%rdi)
123     ret
124     .align 2
...

I'll let Hidenori comment on how to handle this case. I guess the proper way would be to notice the lea of a suspiciously jump-table-like address, but that would mean going back and forth between disassembly, which we don't currently do.

This is actually rather sloppy coding on the part of libffi, since this table could easily be embedded in the rodata section. Why do you need to handle libffi and is it OK to require symbols in this particular case?

@SleepyMug
Copy link

We aren't currently requiring to handle libffi, that was a random experiment I did. And the general assumption (@etherealvisage correct me if I'm wrong) in our case is that we can have the debug symbols if needed.

@etherealvisage
Copy link
Contributor Author

Debug symbols would be a little annoying to require, but certainly regular symbols (i.e. non-stripped binaries) would be fine.

@dwks
Copy link
Contributor

dwks commented Mar 6, 2018

OK. Technically this function is hand-written assembly that violates Egalito's assumptions of reasonable compiler-generated code. So I would rather not add lots of hacks for it. If you are OK with including symbols, and it turns out you need libffi, we can put in a simple hack that skips the basic block at offset 71 during disassembly within the function named ffi_call_unix64@@Base. Everything should then work correctly.

I think we should exit with a clear error message on x86_64 when disassembly fails. It should never really happen except for embedded data like this or unsupported instructions. I'll write a patch and consider this bug closed once that works.

@HidenoriKobayashi
Copy link
Contributor

HidenoriKobayashi commented Mar 7, 2018 via email

@etherealvisage
Copy link
Contributor Author

I think the manual specification of block boundaries that the parse override support enables is sufficient to call this dealt with. As @dwks mentioned it violates the assumption of compiler-generated code, so . . .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants