Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qemu: Add QemuConfig to set qemu args via a struct #2339

Merged
merged 3 commits into from
Aug 14, 2024

Conversation

Marcondiro
Copy link
Contributor

No description provided.

@Marcondiro Marcondiro force-pushed the main branch 7 times, most recently from 3903120 to 44536d8 Compare June 26, 2024 16:45
libafl_qemu/src/qemu/qemu_opt.rs Outdated Show resolved Hide resolved
libafl_qemu/src/qemu/qemu_opt.rs Outdated Show resolved Hide resolved
libafl_qemu/src/qemu/qemu_opt.rs Outdated Show resolved Hide resolved
libafl_qemu/src/qemu/mod.rs Show resolved Hide resolved
libafl_qemu/libafl_qemu_sys/src/systemmode.rs Show resolved Hide resolved
libafl_qemu/build_linux.rs Show resolved Hide resolved
fuzzers/qemu_systemmode/src/fuzzer_classic.rs Outdated Show resolved Hide resolved
@Marcondiro Marcondiro force-pushed the main branch 19 times, most recently from 8ca1cd3 to fd66199 Compare July 4, 2024 08:41
@Marcondiro Marcondiro changed the title qemu: Add QemuOpt to set qemu args via a struct qemu: Add QemuConfig to set qemu args via a struct Jul 4, 2024
@rmalmain
Copy link
Collaborator

rmalmain commented Jul 8, 2024

Also, I just thought about it but we could have a look at the debug flags for systemmode (-s -S).
Last time I tried it didn't work and I thought I would take a look once the typed qemu builder lands because i think we must perform some kind of operation depending on this option.

@Marcondiro Marcondiro force-pushed the main branch 3 times, most recently from ad626a4 to ea428d1 Compare August 2, 2024 14:34
.no_graphic(true)
.snapshot(true)
.drives([qemu_config::Drive::builder()
.interface(qemu_config::DriveInterface::none)
Copy link
Member

@domenukk domenukk Aug 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stupid questions without understanding the details:
Do you actually need to specify none here? And, if so, could Option::None make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you actually need to specify none here?

I'd say yes because, in Qemu, none is not the default interface

And, if so, could Option::None make sense?

mm I don't think so

#[allow(non_camel_case_types)]
#[derive(Debug, strum_macros::Display, Clone)]
#[strum(prefix = "-monitor ")]
pub enum Monitor {
Copy link
Member

@domenukk domenukk Aug 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Monitor and Serial have exactly the same values, It'd make sense to combine them to OutputKind or similar.

Also, what's the difference of none and null?

Also, shouldn't enums be CamelCase?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Monitor and Serial have exactly the same values, It'd make sense to combine them to OutputKind or similar.

It's not clear to me how you would deduplicate it, can you elaborate more?

Also, what's the difference of none and null?

They are 2 different settings in qemu, basically none means no device at all while null is discard its output. https://www.qemu.org/docs/master/system/invocation.html

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes with the prefix hack I guess you can not

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah you could have a common enum and then implement From for the individual enums but it might be overcomplicated without clear benefit

@Marcondiro Marcondiro force-pushed the main branch 3 times, most recently from 8ccc39f to 6398310 Compare August 5, 2024 08:18
@domenukk
Copy link
Member

domenukk commented Aug 5, 2024

@rmalmain
The CI for launcher is unhappy again

    Finished `dev` profile [optimized + debuginfo] target(s) in 4m 06s
[cargo-make] INFO - Running Task: test
+ cd /__w/LibAFL/LibAFL/fuzzers/qemu/qemu_launcher
Profile: dev
+ echo Profile: dev
+ cd injection_test
+ make
gcc -g -o static sqltest.c -l sqlite3 -lm -static -lpthread -ldl
/usr/bin/ld: /usr/lib/gcc/x86_64-linux-gnu/11/../../../x86_64-linux-gnu/libsqlite3.a(os_unix.o): in function `unixDlOpen':
(.text+0x8ad): warning: Using 'dlopen' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
gcc -g -o sqltest sqltest.c -l sqlite3 -lm -lpthread 
+ mkdir in
+ echo aaaaaaaaaa
+ find /__w/LibAFL/LibAFL/target/x86_64 -name qemu_launcher
+ timeout 10s /__w/LibAFL/LibAFL/target/x86_64/debug/qemu_launcher -o out -i in -j ../injections.toml -v -- ./static
+ true
+ grep -Ei found.*injection fuzz.log
+ [ -z  ]
+ echo Fuzzer does not generate any testcases or any crashes
Fuzzer does not generate any testcases or any crashes
+ echo Logs:
Logs:
+ cat fuzz.log
+ exit 1
Error while executing command, exit code: 1
Error: Process completed with exit code 1.

@rmalmain
Copy link
Collaborator

rmalmain commented Aug 5, 2024

looks good to me by now, should we merge once the fix pr is merged?

@rmalmain rmalmain merged commit 21051dc into AFLplusplus:main Aug 14, 2024
98 of 99 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants