AFLplusplus / LibAFL

Advanced Fuzzing Library - Slot your Fuzzer together in Rust! Scales across cores and machines. For Windows, Android, MacOS, Linux, no_std, ...
Other
1.95k stars 297 forks source link

LibAFL QEMU low and high level API separation #1985

Open andreafioraldi opened 4 months ago

andreafioraldi commented 4 months ago

As now the low level is Qemu (called Emulator before) and Emulator is an high level representation to handle sync exits (exit handler + bpts atm), I wonder if the QemuHooks struct (high level handle of hooks) can be either merged to Emulator or that maybe we should better name the things and keep as they are

I see different options:

  1. merge Emulator and QemuHooks, having the low level Qemu object and an higher level one called for instance QemuManager or similar

  2. keep Emulator and QemuHooks separated (both depends on the QemuHelperTuple as generic tho) as can be useful to create an Emulator instance before QemuHooks. We should rename Emulator to QemuRunner or better if you have ideas. A question that arise is if hooks should be able to access the Emulator/QemuRunner, atm they access QemuHooks and Qemu

  3. Completely split the code running in the main including exit handling and the hooks. A problem that the current architecture has is that some Qemu methods should not be called inside a hook. Having different types for when the CPU is not running and another that is used in the hooks parameters (something like Qemu and RunningQemu, CPU and RunningCPU) may solve this problem.

andreafioraldi commented 4 months ago

also, renaming QemuHelper is a good idea IMO, something like QemuTool (like pintool lol) or QemuInstrumentation

domenukk commented 4 months ago

Frida uses Helper extensively as well, so I would either keep the name similar or change it everywhere? (I'm not a fan of the name in general, but it's nice to keep naming consistent)

rmalmain commented 4 months ago

also, renaming QemuHelper is a good idea IMO, something like QemuTool (like pintool lol) or QemuInstrumentation

I also agree QemuHelper is misleading, we should change it IMHO (we have #1784 doing smth like that). QemuTools sounds good to me. I don't know the frida code well but isn't it equivalent to the FridaRuntime trait? If so, the names are already out of sync. We should keep the naming consistent I think.

I also believe we should rename Emulator to something else, I kept it for retro compatibility for now but it's definitively too generic for the purpose it serves. QemuRunner should be a good fit, other propositions: QemuHandler, QemuManager, QemuWorker, QemuOperator.

Still about renaming, I sometimes hear that Backdoor and SyncExit can be misleading names. What is your view on this?

I like the idea of merging Emulator and QemuHooks for multiple reasons:

I think we could still make it possible to create Emulator without hooks, even if it becomes centralized (we could add smth like fn with_hooks and a OnceCell<QemuHooks> for example).

Splitting QEMU into Runtime and non-runtime functions would be neat. It should work even if we choose option 1 no? But maybe this requires a bigger refactoring if we do it.

tokatoka commented 4 months ago

in frida it is runtime

domenukk commented 4 months ago

I don't necessarily agree that Emulator is a bad name: You interact with the emulator that way. It's in the qemu_libafl namespace, so it's also very obvious what it references...