LibAFL icon indicating copy to clipboard operation
LibAFL copied to clipboard

LibAFL QEMU low and high level API separation

Open andreafioraldi opened this issue 1 year ago • 5 comments

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 avatar Mar 30 '24 19:03 andreafioraldi

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

andreafioraldi avatar Mar 30 '24 19:03 andreafioraldi

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)

domenukk avatar Mar 30 '24 19:03 domenukk

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:

  • it centralizes where we use Qemu in one parent struct. Since it's a low-level struct, it's good practice not to split its use imho (for now QemuHooks has a qemu member, it's not ideal)
  • we can then move all the static mut (like EDGE_HOOKS, etc) in the Emulator/ QemuRunner struct directly, it would be much cleaner.
  • As Andrea said, we would centralize the dependency on QemuHelperTuple.
  • Is there a point in keeping QemuHelper inside QemuHooks in case of a merge? I don't feel like there is a dependency relationship between them, maybe it would be better to have them as different fields of Emulator / QemuRunner , and pass a reference to QemuHelperTuple where we need it?

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.

rmalmain avatar Apr 02 '24 08:04 rmalmain

in frida it is runtime

tokatoka avatar Apr 02 '24 20:04 tokatoka

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...

domenukk avatar Apr 03 '24 09:04 domenukk