Repository navigation
fix(intel): resume the gap scan past nop padding after a failed candidate - #381
danielplohmann merged 4 commits into
Conversation
…date The resume added in danielplohmann#338 only looks for int3 runs. GCC and Clang pad ELF functions with nops, so on those binaries a failed gap candidate still dropped the rest of its gap. If the code a failed candidate decoded from its start ends in ret or jmp and is followed only by nops up to a 16-byte boundary, resume there. 18 static x64 ELF builds (lz4, zstd, brotli; gcc/clang; O0/O2/Os), scored against their symbol tables: +2 true positives, +1 false positive. Malpedia CI corpus: 152/155 files unchanged, one Akira ELF +109 functions, two Konni samples +1 each, nothing lost.
ty cannot see the None check in _failedGapResumeTarget from inside the helpers it calls, so self.gap_pointer + 1 failed type checking there.
danielplohmann
left a comment
There was a problem hiding this comment.
Thanks. Since "ret/jmp + nops up to 16 bytes" also marks aligned loop heads inside GCC functions, I wanted numbers on a ground truth before saying anything, so I ran master and this branch over our multiplatform ground truth: 866 unstripped x86/x64 ELFs (busybox, dropbear, inetutils, lighttpd; GCC 4.9.2 / 8.1 and clang, -O2/-O3, none of them CET), each stripped of its symbols, and scored function starts against the original symbol table.
| true starts | false starts | |
|---|---|---|
| master | 123,022 | 59,558 |
| this PR | 123,484 | 59,554 |
| difference | +464 / -2 | +2 / -6 |
With .eh_frame stripped as well: +454 / -2 true, +3 / -6 false. 133 of 866 files change, all of them GCC builds (dropbear, inetutils, lighttpd; i686 more than x86_64). No clang build changes. So the loop-head concern does not show up in practice: the resume only happens after a failed candidate, and on this data those failures sit between functions far more often than inside them. Happy to merge this.
The first instructions of the 464 new true starts are push (298), sub (103), mov (43) and repz ret (20), so a prologue check at the resume target would throw away over a third of the gain on pre-CET code. Please don't add one.
Small things, none blocking:
_straightLineEndandfailed_gap_extent_endlive in the sharedRecursiveDisassembler/FunctionCandidateManager, so they run after every failed candidate on every backend, sorting the state's instructions each time, while only the intel manager reads the result. Could the computation move into the intel path, or be skipped when the manager doesn't use it?_failedGapResumeTargetcan return early (no gap pointer, no binary info, negative offset) beforefailed_gap_extent_endis cleared, so an old value can stay around. Theend <= gap_pointercheck keeps this harmless today, but clearing it at the top of the function would be cleaner.- Only capstone's
nopcounts as padding. Old i386 binutils fillers (lea esi,[esi+0],mov esi,esi) aren't covered. I understand why (Delphilea eax,[eax]), but a sentence in the CHANGELOG would help. - There will be a CHANGELOG conflict with #380 once that one lands.
…ger only The extent was computed in the shared gap loop after every failed candidate on every backend, sorting the state's instructions each time, while only the x86 manager reads it. Hand the state to a manager hook that is a no-op by default, walk the decoded run through an address map instead of a sort, and drop the stored extent at the top of the resume so an early return cannot leave it behind. The CHANGELOG entry now notes that old i386 fillers are not read as padding and carries the reviewer's ground-truth numbers.
|
Thanks for running it over the ground truth. I'll leave the resume target without a prologue check. All four small points are addressed in deb7555:
Function sets are unchanged against f5290cb on 18 static x64 ELF builds and the Go reproducer from #363. |
…me-without-int3 # Conflicts: # CHANGELOG.md
The failed-candidate resume from #338 only looks for int3 runs. GCC and Clang pad ELF functions with nops, so on those binaries a gap candidate that fails still drops the rest of its gap. On an Akira ELF from the Malpedia corpus, a 7-byte tail fragment at the start of a 3.9 KB gap fails, and the 17 functions behind it are never offered.
Change: when the code a failed candidate decoded from its start ends in
ret/jmpand only nops follow up to a 16-byte boundary, the scan resumes at that boundary. A run that ends anywhere else, or reaches an aligned address with no padding, keeps the old behaviour. Requiring both is what keeps misdecoded candidates on Delphi code (4-byte aligned, padded withlea eax, [eax]) from producing resume points inside functions; without it the Malpedia corpus picked up ~400 such hits.Measured:
make test-allpasses and the bundled fixtures do not move. New tests intests/testFailedGapResume.pycover the resume target and an end-to-end image where an unreferenced function sits behind a failing fragment and nop padding.