Frida Refactor: Split FridaHelper into each Runtime #368

tokatoka · 2021-11-08T01:19:05Z

This pr includes several frida refactoring
#307

Split coverage runtime
Test builing for aarch64
Clean up

tokatoka · 2021-11-08T02:40:16Z

I think I should split helper, and make a CoverageRuntime
just like AsanRuntime, CmpLogRuntime

s1341 · 2021-11-08T03:49:43Z

libafl_frida/src/helper.rs

+    fn generate_maybe_log_blob(&mut self) -> Box<[u8]> {
+        macro_rules! maybe_log{
+            ($ops:ident) => {dynasm!($ops
+                ;   .arch x64


We don’t really need the macro, do we?

No 😉 we don't need macro
I plan to move this to CoverageRuntime.

s1341 · 2021-11-08T03:54:48Z

Yes splitting out CoverageRuntime is a good idea. It might also be worth moving the is_interesting functions out of helper into their various Runtimes.

s1341 · 2021-11-08T03:56:22Z

Also it doesn’t look like you used your new function anywhere, did you? It should be called only once and the result cached.

tokatoka · 2021-11-08T04:00:47Z

no, I can't use it yet.
that's why it's still a draft.
At first I tried to call this from FridaHelper->new(), but this does not look good because we can't call self.generate_maybe_log_blob to slot the blob into self in new().
(what I can do is like calling FridaInstrumentationHelper::<'a>::generate_maybe_log_blob(&mut helper) but that doesn't look nice)
To achieve this, I find it's nicer to create a CoverageRuntime and we can call that generate-blob function in something like CoverageRuntime->init()

s1341 · 2021-11-08T04:17:04Z

Cool. Makes sense. I’ll let you work. ;)

tokatoka · 2021-11-09T13:57:15Z

This change already works on amd64
i'm gonna rewrite the aarch64 part and test building for aarch64 tomorrow
(I hope we have an android ci tbh...)

tokatoka · 2021-11-09T14:01:47Z

how about moving hook_funcs to another file like asan_func_hooks.rs? @s1341
currently it is just... toooooo long

s1341 · 2021-11-09T17:57:38Z

Good idea. Do you want to do it in this PR or in another?

tokatoka · 2021-11-09T17:58:23Z

The fix would be simple. I would do it in this pr.

s1341 · 2021-11-09T18:01:22Z

Ok. Go ahead.

s1341 · 2021-11-09T18:01:43Z

Let me know when you want me to test on aarch64.

This reverts commit c7afc78.

tokatoka · 2021-11-10T11:27:21Z

I come to feel that I should do these in seperate prs...

tokatoka · 2021-11-10T11:28:39Z

so that I can track down my mistakes if I broke something.

tokatoka · 2021-11-10T16:39:09Z

I checked building for aarch64.

tokatoka · 2021-11-10T16:40:09Z

I would like to get this merged and work on remaining issues in other prs

s1341

A few nitpicks... but otherwise lgtm!

s1341 · 2021-11-11T06:25:16Z

libafl_frida/src/coverage_rt.rs

+
+pub struct CoverageRuntime {
+    map: [u8; MAP_SIZE],
+    previous_pc: [u64; 1],


Why is this an array?

it was an array before ;)
but i'll fix

Ok. Didn't realize... Yeah if you can fix...

s1341 · 2021-11-11T06:26:25Z

libafl_frida/src/coverage_rt.rs

+        self.generate_maybe_log_blob();
+    }
+
+    pub fn map_ptr(&mut self) -> *mut u8 {


shouldn't this be map_ptr_mut to stick to the normal rust conventions?

s1341 · 2021-11-11T06:28:01Z

libafl_frida/src/coverage_rt.rs

+            ;   ldr x2, #0x38
+            ;   ldr x4, [x2]
+            ;   eor x4, x4, x0
+            ;   and x4, x4, 0xffff


This should be MAP_SIZE - 1 instead of 0xffff

s1341 · 2021-11-11T06:30:02Z

libafl_frida/src/coverage_rt.rs

+            ;   stp x1, x2, [sp, -0x10]!
+            ;   stp x3, x4, [sp, -0x10]!
+            ;   ldr x1, >map_addr
+            ;   ldr x2, #0x38


This should be >previous_loc

tokatoka · 2021-11-11T07:16:38Z

I fixed all, but didn't test how aarch64 build goes with the change.

s1341 · 2021-11-11T07:37:24Z

I fixed a minor issue which was breaking aarch64 compilation, but running on aarch64 blows up for some reason. I'm debugging now.

domenukk · 2021-11-11T07:42:02Z

Can you maybe try to add the build to CI whole you do?

s1341 · 2021-11-11T07:43:08Z

It requires adding/installing an ndk... which I don't know how to do under CI in a way that won't be very time-expensive.

domenukk · 2021-11-11T07:45:48Z

We already build parts of it, see

LibAFL/.github/workflows/build_and_test.yml

Line 209 in 9ab8663

run: cargo ndk -t arm64-v8a build --release

s1341 · 2021-11-11T09:03:30Z

Hrm. I'll try to look into it.

tokatoka · 2021-11-11T12:48:26Z

I fixed a minor issue which was breaking aarch64 compilation, but running on aarch64 blows up for some reason. I'm debugging now.

I observe that arm64 run is broken on main branch too. (with segmentation fault)

tokatoka · 2021-11-11T13:05:28Z

mine fails at this

Thread 3.1 "frida_libpng" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xfffff7c03430 (LWP 41363)]
0x0000aaaaaad30880 in nix::sys::mman::mmap (addr=0x400000000000, length=<optimized out>, prot=..., flags=..., fd=-1, offset=0)
    at /home/ubuntu/.cargo/registry/src/github.com-1ecc6299db9ec823/nix-0.23.0/src/sys/mman.rs:339
339         let ret = libc::mmap(addr, length, prot.bits(), flags.bits(), fd, offset);

but I'm not turning asan on.... 🤔

s1341 · 2021-11-11T13:07:30Z

I think the coverage is broken. Haven't had a moment to debug today... Hopefully will get to it a little later.

tokatoka · 2021-11-11T13:08:57Z

I confirmed that this error already occurs back at f0daeb3 this commit, (just before amd64 asan)

s1341 · 2021-11-11T13:10:18Z

Is it possible that the machine you are running on doesn't have memory over-commit?

tokatoka · 2021-11-11T13:12:23Z

Oh, ... it is possible.
It is just a potato aws instance...
ok, so my comments would be unrelated, sorry.

domenukk · 2021-11-11T13:30:17Z

I don't think it's possible, they should be normal Linux OSses with normal configuration.

BTW @tokatoka in case you are spawning instances mainly for libafl dev, let us know, I'm sure we have some leftover gsoc funds for that or something :)

tokatoka · 2021-11-11T13:31:41Z

it's ok.
yesterday I used that for like 2 hours but just costed me 10 jpy (like less than 0.1 euro)

andreafioraldi · 2021-11-12T11:59:46Z

Should I merge?

tokatoka · 2021-11-12T12:01:00Z

no aarch64 run is broken, let's wait s1341

* dynasm maybe_log * create coverage_rt, trim helper * add * amd64 working * aarch64 instrumentation, untested * asan dir * Revert "asan dir" This reverts commit c7afc78. * non x86_64 fix * clippy * change * change * fix * Fix aarch64-linux-android build * Fix aarch64 execution * Fix fmt Co-authored-by: s1341 <github@shmarya.net>

dynasm maybe_log

e153074

tokatoka changed the title ~~Dynasm maybe_log~~ Frida Refactor: Split CoverageRuntime from FridaHelper Nov 8, 2021

s1341 reviewed Nov 8, 2021

View reviewed changes

tokatoka added 3 commits November 9, 2021 22:35

create coverage_rt, trim helper

1b8c8a4

add

ca0c555

amd64 working

79e8d1c

tokatoka marked this pull request as ready for review November 9, 2021 13:58

aarch64 instrumentation, untested

92b27c6

tokatoka changed the title ~~Frida Refactor: Split CoverageRuntime from FridaHelper~~ Frida Refactor: Split FridaHelper into each Runtime Nov 9, 2021

tokatoka added 2 commits November 10, 2021 20:03

asan dir

c7afc78

Revert "asan dir"

730f740

This reverts commit c7afc78.

tokatoka added 2 commits November 11, 2021 01:36

non x86_64 fix

b05a7ea

clippy

aade663

tokatoka requested a review from s1341 November 10, 2021 16:39

s1341 reviewed Nov 11, 2021

View reviewed changes

tokatoka mentioned this pull request Nov 11, 2021

Frida Refactor: Frida executor #374

Merged

tokatoka added 3 commits November 11, 2021 16:00

change

47f3ef6

change

89bcf28

fix

5f7eaa3

Fix aarch64-linux-android build

117ad02

domenukk requested a review from s1341 November 14, 2021 21:42

tokatoka and others added 3 commits November 16, 2021 16:54

Merge branch 'main' into dynasm_coverage

3fb2c11

Fix aarch64 execution

96aafb8

Fix fmt

42210e8

domenukk merged commit c7512fc into main Nov 16, 2021

tokatoka deleted the dynasm_coverage branch November 16, 2021 19:50

Frida Refactor: Split FridaHelper into each Runtime #368

Frida Refactor: Split FridaHelper into each Runtime #368

Conversation

tokatoka commented Nov 8, 2021 • edited

tokatoka commented Nov 8, 2021 • edited

s1341 Nov 8, 2021 • edited

Choose a reason for hiding this comment

tokatoka Nov 8, 2021 • edited

Choose a reason for hiding this comment

s1341 commented Nov 8, 2021

s1341 commented Nov 8, 2021

tokatoka commented Nov 8, 2021 • edited

s1341 commented Nov 8, 2021

tokatoka commented Nov 9, 2021 • edited

tokatoka commented Nov 9, 2021 • edited

s1341 commented Nov 9, 2021

tokatoka commented Nov 9, 2021

s1341 commented Nov 9, 2021

s1341 commented Nov 9, 2021

tokatoka commented Nov 10, 2021

tokatoka commented Nov 10, 2021

tokatoka commented Nov 10, 2021 • edited

tokatoka commented Nov 10, 2021

s1341 left a comment

Choose a reason for hiding this comment

s1341 Nov 11, 2021

Choose a reason for hiding this comment

tokatoka Nov 11, 2021

Choose a reason for hiding this comment

s1341 Nov 11, 2021

Choose a reason for hiding this comment

s1341 Nov 11, 2021

Choose a reason for hiding this comment

s1341 Nov 11, 2021

Choose a reason for hiding this comment

s1341 Nov 11, 2021

Choose a reason for hiding this comment

tokatoka commented Nov 11, 2021

s1341 commented Nov 11, 2021

domenukk commented Nov 11, 2021

s1341 commented Nov 11, 2021

domenukk commented Nov 11, 2021

s1341 commented Nov 11, 2021

tokatoka commented Nov 11, 2021 • edited

tokatoka commented Nov 11, 2021

s1341 commented Nov 11, 2021

tokatoka commented Nov 11, 2021

s1341 commented Nov 11, 2021

tokatoka commented Nov 11, 2021

domenukk commented Nov 11, 2021

tokatoka commented Nov 11, 2021

andreafioraldi commented Nov 12, 2021

tokatoka commented Nov 12, 2021

tokatoka commented Nov 8, 2021 •

edited

tokatoka commented Nov 8, 2021 •

edited

s1341 Nov 8, 2021 •

edited

tokatoka Nov 8, 2021 •

edited

tokatoka commented Nov 8, 2021 •

edited

tokatoka commented Nov 9, 2021 •

edited

tokatoka commented Nov 9, 2021 •

edited

tokatoka commented Nov 10, 2021 •

edited

tokatoka commented Nov 11, 2021 •

edited