Refactor of Qemu configuration #2707

rmalmain · 2024-11-19T15:29:45Z

No description provided.

libafl_qemu/src/qemu/config.rs

domenukk · 2024-11-19T17:23:52Z

libafl_qemu/src/qemu/config.rs


    #[test]
    #[cfg(feature = "usermode")]
    fn usermode() {
        let program = "/bin/pwd";
-        let qemu = Qemu::builder().program("/bin/pwd").build().unwrap();


Doens't the old API look better here? Can't we just keep Qemu::builder() and .build() then calls Qemu::init_whatever?

That's true, but the thing is that we need to separate the configuration step from the init step, and the builder generated by typed-builder is not made for this, it's a pain to move around

IMHO it's more important to have a good API than to have nice code "on the inside".
This is already what you're using (right?) idanarye/rust-typed-builder#80

from the user's point of view, i think separating configuration from actual initialization is not so bad as well.
it makes more clear what the configuration part is and where qemu is actually getting initialized.

hiding something like qemu initialization in a From implementation doesn't sound good imho

No it's really bad for users. We have builders everywhere in the codebase and they all work the same. There's no reason this shoudl work differently. You can still pass around an un-built builder

but it works like any other builder, right? it's just that the builder doesn't build directly qemu, but qemu's config instead.
i think QemuConfig::builder makes it quite clear what we are building is not QEMU, right?

libafl_qemu/src/qemu/config.rs

Marcondiro · 2024-11-20T13:08:54Z

thread '<unnamed>' panicked at /home/marco/code/LibAFL/libafl_qemu/src/emu/hooks.rs:926:18:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0:     0x593b8f62ddea - std::backtrace_rs::backtrace::libunwind::trace::h5f6f9a5bb923a5c4
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/std/src/../../backtrace/src/backtrace/libunwind.rs:116:5
   1:     0x593b8f62ddea - std::backtrace_rs::backtrace::trace_unsynchronized::h9536729b6037ebc1
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x593b8f62ddea - std::sys::backtrace::_print_fmt::hb9e02cd4bef90f24
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/std/src/sys/backtrace.rs:66:9
   3:     0x593b8f62ddea - <std::sys::backtrace::BacktraceLock::print::DisplayBacktrace as core::fmt::Display>::fmt::hd1943a7dbe21bbdb
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/std/src/sys/backtrace.rs:39:26
   4:     0x593b8f659393 - core::fmt::rt::Argument::fmt::h4e8e7a99a23a88d6
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/core/src/fmt/rt.rs:177:76
   5:     0x593b8f659393 - core::fmt::write::hde915a39d87f3313
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/core/src/fmt/mod.rs:1189:21
   6:     0x593b8f629533 - std::io::Write::write_fmt::hc2b9c2b5d6f1f693
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/std/src/io/mod.rs:1884:15
   7:     0x593b8f62dc32 - std::sys::backtrace::BacktraceLock::print::hdce7620bc0dfaf85
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/std/src/sys/backtrace.rs:42:9
   8:     0x593b8f62ef7d - std::panicking::default_hook::{{closure}}::h10da497c654479af
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/std/src/panicking.rs:268:22
   9:     0x593b8f62edc3 - std::panicking::default_hook::h4fe79fc5adc3f2a3
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/std/src/panicking.rs:295:9
  10:     0x593b8f62f5b7 - std::panicking::rust_panic_with_hook::he2ffe4493a7180a3
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/std/src/panicking.rs:801:13
  11:     0x593b8f62f416 - std::panicking::begin_panic_handler::{{closure}}::h284c81a6cd3618b7
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/std/src/panicking.rs:667:13
  12:     0x593b8f62e2c9 - std::sys::backtrace::__rust_end_short_backtrace::h0e39815e4ecdc98d
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/std/src/sys/backtrace.rs:170:18
  13:     0x593b8f62f0dc - rust_begin_unwind
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/std/src/panicking.rs:665:5
  14:     0x593b8f6563e0 - core::panicking::panic_fmt::h436516364a9f930c
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/core/src/panicking.rs:76:14
  15:     0x593b8f65646c - core::panicking::panic::h0dba2e871100d1ff
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/core/src/panicking.rs:148:5
  16:     0x593b8f656359 - core::option::unwrap_failed::h807e9a496db40526
                               at /rustc/03ee4845197ce71aa5ee28cb937a3e863b18b42f/library/core/src/option.rs:2009:5
  17:     0x593b8e74453a - core::option::Option<T>::unwrap::h6bde8973e9021d1d
                               at /home/marco/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:972:21
  18:     0x593b8e74453a - libafl_qemu::emu::hooks::EmulatorModules<ET,S>::emulator_modules_mut_unchecked::h1c80e3484aeffb15
                               at /home/marco/code/LibAFL/libafl_qemu/src/emu/hooks.rs:924:13
  19:     0x593b8e6d7245 - libafl_qemu::qemu::hooks::func_new_thread_hook_wrapper::hce646f9fae116813
                               at /home/marco/code/LibAFL/libafl_qemu/src/qemu/hooks.rs:136:35
  20:     0x593b8ecf9de4 - libafl_hook_new_thread_run
                               at /home/marco/code/qemu-libafl-bridge/build/../libafl/hooks/thread.c:42:17
  21:     0x593b8ed66051 - kvm_vcpu_thread_fn
                               at /home/marco/code/qemu-libafl-bridge/build/../accel/kvm/kvm-accel-ops.c:52:5
  22:     0x593b8ef1c730 - qemu_thread_start
                               at /home/marco/code/qemu-libafl-bridge/build/../util/qemu-thread-posix.c:541:9
  23:     0x77447589ca94 - start_thread
                               at ./nptl/pthread_create.c:447:8
  24:     0x774475929c3c - clone3
                               at ./misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:78
  25:                0x0 - <unknown>

Marcondiro · 2024-11-25T09:31:32Z

LibAFL/libafl_qemu/src/emu/builder.rs

Lines 127 to 130 in af20343

    
           let qemu = Qemu::init_with_params(&qemu_parameters)?; 
        
           unsafe { 
        
               Ok(Emulator::new_with_qemu(

@rmalmain the problem is that Qemu::init_with_params is run before EMULATOR_MODULES is initialized by Emulator::new_with_qemu

* back to one QEMU init function * other small things

* qemu is now a parameter to EmulatorModule callbacks and most function hooks. * EmulatorModules is initialized before QEMU is initialized.

rmalmain · 2024-11-26T15:50:34Z

some things we should do at some point:

i think we should make Emulator an argument to EmulatorModule and remove the self reference. otherwise (it's an old problem) each EmulatorModule has a double mut reference to itself (one through &mut self and one through EmulatorModules).
we should find a better way to type hooks callbacks. for now they are declared 2 times (both in rust and c side) and can easily get messed up. also, it's quite verbose to create new hooks. imho we should create some kind of builder on c side and simplify the rust side with more macros.

* continue to propagate qemu argument as hook first parameter * use pre_syscall* and post_syscall* everywhere * fix some clippy stuff

…d use the module interface instead. * adapt qemu_launcher example to fully work with emulator, since qemu must now be initialized by emulator.

rmalmain · 2024-11-27T18:38:35Z

this pr has an important breaking change: qemu cannot be initialized independently from emulator. if emulator is needed, qemu must be initialized by emulator (before it was possible to initialize qemu and give it to emulator).
this is because we now have callbacks for things happening before qemu gets initialized, so obviously we cannot initialize it before.
i changed the way asan stuff gets initialized, it should be more natural (i removed the custom init of qemu for asan and moved it to module callbacks).

qemu config refactoring

af20343

Marcondiro reviewed Nov 19, 2024

View reviewed changes

libafl_qemu/src/qemu/config.rs Outdated Show resolved Hide resolved

domenukk reviewed Nov 19, 2024

View reviewed changes

Marcondiro reviewed Nov 20, 2024

View reviewed changes

libafl_qemu/src/qemu/config.rs Outdated Show resolved Hide resolved

Marcondiro reviewed Nov 20, 2024

View reviewed changes

libafl_qemu/src/qemu/config.rs Show resolved Hide resolved

Merge branch 'main' into qemu_builder_update

5fa831a

rmalmain added 3 commits November 25, 2024 11:58

* QEMU error refactoring*

28655d8

* back to one QEMU init function * other small things

update test

b85ab06

Light refactor of EmulatorModules

47d2741

* qemu is now a parameter to EmulatorModule callbacks and most function hooks. * EmulatorModules is initialized before QEMU is initialized.

rmalmain added 10 commits November 26, 2024 17:02

Merge branch 'main' into qemu_builder_update

0d37b78

fmt

1c61b38

* asan fixed size accesses working with generics

525fd11

* continue to propagate qemu argument as hook first parameter * use pre_syscall* and post_syscall* everywhere * fix some clippy stuff

fmt

5346f83

fix fuzzers

17059d7

* refactor asan and asanguest modules to avoid custom init of QEMU an…

c50ba05

…d use the module interface instead. * adapt qemu_launcher example to fully work with emulator, since qemu must now be initialized by emulator.

fmt

8d8631e

clippy

f2491c1

Merge branch 'main' into qemu_builder_update

a12dc91

fix qemu_coverage

76480c1

fmt

46f0da7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor of Qemu configuration #2707

Refactor of Qemu configuration #2707

rmalmain commented Nov 19, 2024

domenukk Nov 19, 2024

Marcondiro Nov 20, 2024

domenukk Nov 20, 2024

rmalmain Nov 22, 2024

domenukk Nov 22, 2024 •

edited

Loading

rmalmain Nov 25, 2024

Marcondiro commented Nov 20, 2024

Marcondiro commented Nov 25, 2024 •

edited

Loading

rmalmain commented Nov 26, 2024

rmalmain commented Nov 27, 2024

Refactor of Qemu configuration #2707

Are you sure you want to change the base?

Refactor of Qemu configuration #2707

Conversation

rmalmain commented Nov 19, 2024

domenukk Nov 19, 2024

Choose a reason for hiding this comment

Marcondiro Nov 20, 2024

Choose a reason for hiding this comment

domenukk Nov 20, 2024

Choose a reason for hiding this comment

rmalmain Nov 22, 2024

Choose a reason for hiding this comment

domenukk Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

rmalmain Nov 25, 2024

Choose a reason for hiding this comment

Marcondiro commented Nov 20, 2024

Marcondiro commented Nov 25, 2024 • edited Loading

rmalmain commented Nov 26, 2024

rmalmain commented Nov 27, 2024

domenukk Nov 22, 2024 •

edited

Loading

Marcondiro commented Nov 25, 2024 •

edited

Loading