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 launcher bugfix #2858

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

noobone123
Copy link
Contributor

  1. Fixed the issue where compilation failed under the simplemgr features in Cargo.toml
  2. Fixed the issue where the crash handler would fail during rerun. This issue was discovered with the guidance of @tokatoka , and its root cause is quite complex, as detailed in Strange behavior with the AsanModule in LibAFL_qemu #2837.
    (P.S. Currently, another issue potentially related to the AsanModule has been observed and may be further discussed in Strange behavior with the AsanModule in LibAFL_qemu #2837.)

@@ -241,9 +241,9 @@ impl<M: Monitor> Instance<'_, M> {

executor
.run_target(
&mut NopFuzzer::new(),
&mut fuzzer,
Copy link
Member

@tokatoka tokatoka Jan 16, 2025

Choose a reason for hiding this comment

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

this is the problem of the bug.

remember the function enter_target. where you write the pointer to the executor, state, eventmgr, fuzzer, and input to the global.

The types of those are the ones passed into the argument here. so nopfuzzer and nopeventmanager

Copy link
Member

@tokatoka tokatoka Jan 16, 2025

Choose a reason for hiding this comment

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

however when you recover from the signal. you call inproc crash handler and run_observers_and_save_state
but the types used in that function is the one used when you define them. thus stdfuzzzer and the normal event manager

so this is a type confusion

Copy link
Member

@tokatoka tokatoka Jan 16, 2025

Choose a reason for hiding this comment

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

IMO the solution is to forcing every inprocess executor to hold the type S and Z (by adding them to phantom data)

Copy link
Member

Choose a reason for hiding this comment

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

actually i think this is the first time i saw that our unsafe things in inprocess executor is causing a problem...

Copy link
Member

Choose a reason for hiding this comment

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

Can it be related to the type refactoring or something? Or recent changes otherwise?

Copy link
Member

@tokatoka tokatoka Jan 17, 2025

Choose a reason for hiding this comment

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

no no ... this happened before he merged those changes i think...
and i just explained above how it happened.

1 .

when you recover from the signal. you call inproc crash handler and run_observers_and_save_state
but the types used in that function is the one used when you define them.

remember the function enter_target. where you write the pointer to the executor, state, eventmgr, fuzzer, and input to the global.
The types of those are the ones passed into the argument here. so nopfuzzer and nopeventmanager

You write the pointer of an object in 1. thinking this is the type Z. but when you recover the pointer in 2., the rust code thinks it is the type of Z'

.map(|h| h.tokens.clone())
.unwrap_or_default()
} else {
Vec::new()
Copy link
Member

Choose a reason for hiding this comment

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

@rmalmain is there a better option for this? .maybe_extra_token that takes a Some or so?
Also, we should find a way to add CI for this

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.

3 participants