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

Multi machine follow-up #2334

Merged
merged 44 commits into from
Sep 4, 2024
Merged

Multi machine follow-up #2334

merged 44 commits into from
Sep 4, 2024

Conversation

rmalmain
Copy link
Collaborator

No description provided.

libafl/src/events/mod.rs Outdated Show resolved Hide resolved
libafl/src/state/mod.rs Outdated Show resolved Hide resolved
@tokatoka
Copy link
Member

idk but this doesn't work with fuzzers/fuzzbench

@tokatoka
Copy link
Member

fatal runtime error: failed to initiate panic, error 5
thread '' panicked at src/lib.rs:238:17:
Failed to setup the restarter: Error in Serialization: DeserializeBadVarint

@rmalmain
Copy link
Collaborator Author

from which fuzzer exactly you get this error?

@tokatoka
Copy link
Member

fuzzers/fuzzbench
then make libafl incldue dump_state feature

@rmalmain
Copy link
Collaborator Author

i check

@domenukk
Copy link
Member

You could consider dumping to toml or something more human firendly(?)

@@ -322,6 +371,8 @@ where
hit_feedbacks: Vec::new(),
#[cfg(feature = "track_hit_feedbacks")]
hit_objectives: Vec::new(),
#[cfg(feature = "dump_state")]
timestamp: current_time(),
Copy link
Member

Choose a reason for hiding this comment

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

Why not current_milliseconds for no_std? Also, do we maybe always want this?

use libafl_bolts::os::unix_signals::{siginfo_t, ucontext_t, Handler, Signal, CTRL_C_EXIT};
use libafl_bolts::os::unix_signals::{siginfo_t, ucontext_t, Handler, Signal};
#[cfg(all(unix, feature = "std", not(feature = "dump_state")))]
use libafl_bolts::os::CTRL_C_EXIT;
Copy link
Member

Choose a reason for hiding this comment

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

STATUS_CTRL_C_EXIT?

@@ -51,6 +53,8 @@ use uuid::Uuid;

#[cfg(feature = "introspection")]
use crate::state::HasClientPerfMonitor;
#[cfg(all(unix, feature = "dump_state"))]
use crate::INTERRUPT_FUZZER;
Copy link
Member

Choose a reason for hiding this comment

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

SHOULD_STOP or something?

type StateDump: Serialize + for<'de> Deserialize<'de>;

/// Get the dump dir, if there is one.
fn dump_state_dir(&self) -> Option<&PathBuf>;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be a specific file?

#[cfg(all(feature = "std", feature = "dump_state"))]
#[derive(Serialize, Deserialize, Clone, Debug)]
#[serde(bound = "I: serde::de::DeserializeOwned")]
pub struct StdStateDump<I>
Copy link
Member

Choose a reason for hiding this comment

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

Yeah... this makes no sense anymore - just write the state to disk using serde_json::to_string oder whatever

tcs.push(tc_ref.clone().try_into()?);
}

Ok(StdStateDump {
Copy link
Member

Choose a reason for hiding this comment

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

Why not the dump the whole state now?

Copy link
Member

Choose a reason for hiding this comment

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

at this point we are doing this just for our experiment

Copy link
Member

Choose a reason for hiding this comment

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

but to answer the question,
we wanted to parse this in a standalone tool where modules such as Scheduler does not exist, but this cause a problem because we have to add types to it to compile our tool.

That's why we just select a few members from it and make it into another struct (but again, it's just for the experiment

Copy link
Member

@domenukk domenukk Jul 1, 2024

Choose a reason for hiding this comment

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

You can dump to JSON and remove the keys you don't want
or maybe you can change the deserializer to ignore unsupported types

Copy link
Member

Choose a reason for hiding this comment

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

yes dumping to json is the best option instead of the serialized bytes of states..

@domenukk domenukk marked this pull request as draft July 1, 2024 14:37
@tokatoka
Copy link
Member

tokatoka commented Jul 2, 2024

i think we won't merge this in the end?

@domenukk
Copy link
Member

domenukk commented Jul 3, 2024

Having a proper state dump (like, a full one) could be good?

@rmalmain
Copy link
Collaborator Author

there are changes there that should be merged IMHO.
I'll do a clean version of dump_state, i think it's good to have this around

@tokatoka
Copy link
Member

i think dump_state should dump the state in a human-readable format (json or toml)
and it does not need to dump the testcase, because we already have a stage for it.

@rmalmain
Copy link
Collaborator Author

i agree of the human-readable format.
we still need a form of link between this and the tcs no?
for example we can store the hash of tcs in the json stuff per fuzzing core or smth

@tokatoka tokatoka marked this pull request as ready for review September 3, 2024 13:44
@tokatoka
Copy link
Member

tokatoka commented Sep 4, 2024

if the purpose is to dump the state, there's more proper way to do it without using signals.
i'll keep the really necessary part of this and merge

@tokatoka tokatoka merged commit 203d3d3 into main Sep 4, 2024
21 of 22 checks passed
@tokatoka tokatoka deleted the multi_machine_2 branch September 4, 2024 16:42
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