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

libafl_nyx: Allow custom input buffer size to be passed to NyxHelper #1960

Merged
merged 9 commits into from
Mar 21, 2024

Conversation

l4yton
Copy link
Contributor

@l4yton l4yton commented Mar 21, 2024

Initially, I only wanted to add the option to specify the input buffer size, but I ended up changing a couple things:

  • New NyxSettings struct that is passed to NyxHelper::new.
  • NyxExecutor takes ownership of NyxHelper instead of taking a mutable reference, this removes some lifetime complexity and I didn't see any reason against it.
  • Remove dry run in NyxHelper. If QEMU didn't spawn, it should be noticed by NyxExecutor anyway.

Let me know if this makes sense and what you think.

Copy link
Member

@domenukk domenukk left a comment

Choose a reason for hiding this comment

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

Looks good!

@@ -14,7 +14,7 @@ categories = ["development-tools::testing", "emulators", "embedded", "os", "no-s
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[target.'cfg(target_os = "linux")'.dependencies]
libnyx = {git = "https://github.com/nyx-fuzz/libnyx.git",rev = "acaf7f6"}
libnyx = { git = "https://github.com/nyx-fuzz/libnyx.git", rev = "acaf7f6" }
Copy link
Member

Choose a reason for hiding this comment

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

Should we upgrade this to the latest rev?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started working on using the latest libnyx revision here: https://github.com/l4yton/LibAFL/tree/libnyx_latest, but will have to do some more testing. Documentation and example fuzzers would also have to be updated since NyxProcess gets the config directly from the config.ron file in the share_dir.

libafl_nyx/src/executor.rs Outdated Show resolved Hide resolved
libafl_nyx/src/settings.rs Show resolved Hide resolved
libafl_nyx/src/settings.rs Outdated Show resolved Hide resolved
@domenukk
Copy link
Member

I left a few comments, and Clippy is still unhappy:

error: redundant closure
  --> libafl_nyx/src/helper.rs:79:18
   |
79 |         .map_err(|e| libafl::Error::illegal_argument(e))?;
   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `libafl::Error::illegal_argument`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
   = note: `-D clippy::redundant-closure` implied by `-D clippy::all`
   = help: to override `-D clippy::all` add `#[allow(clippy::redundant_closure)]`

error: this method could have a `#[must_use]` attribute
  --> libafl_nyx/src/settings.rs:20:5
   |
20 | /     pub fn new(
21 | |         cpu_id: u32,
22 | |         parent_cpu_id: Option<u32>,
23 | |         snap_mode: bool,
24 | |         parallel_mode: bool,
25 | |     ) -> Self {
   | |_____________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#must_use_candidate
   = note: `-D clippy::must-use-candidate` implied by `-D clippy::pedantic`
   = help: to override `-D clippy::pedantic` add `#[allow(clippy::must_use_candidate)]`
help: add the attribute
   |
20 ~     #[must_use] pub fn new(
21 +         cpu_id: u32,
22 +         parent_cpu_id: Option<u32>,
23 +         snap_mode: bool,
24 +         parallel_mode: bool,
25 ~     ) -> Self {
   |

error: this method could have a `#[must_use]` attribute
  --> libafl_nyx/src/settings.rs:39:5
   |
39 |     pub fn with_cpu_id(mut self, cpu_id: u32) -> Self {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[must_use] pub fn with_cpu_id(mut self, cpu_id: u32) -> Self`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#must_use_candidate

error: missing `#[must_use]` attribute on a method returning `Self`

error: missing `#[must_use]` attribute on a method returning `Self`
  --> libafl_nyx/src/settings.rs:64:5
   |
64 | /     pub fn with_timeout_secs(mut self, timeout_secs: u8) -> Self {
65 | |         self.timeout_secs = timeout_secs;
66 | |         self
67 | |     }
   | |_____^
   |
   = help: consider adding the `#[must_use]` attribute to the method or directly to the `Self` type
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#return_self_not_must_use

error: this method could have a `#[must_use]` attribute
  --> libafl_nyx/src/settings.rs:69:5
   |
69 |     pub fn with_timeout_micro_secs(mut self, timeout_micro_secs: u32) -> Self {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[must_use] pub fn with_timeout_micro_secs(mut self, timeout_micro_secs: u32) -> Self`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#must_use_candidate

error: missing `#[must_use]` attribute on a method returning `Self`
  --> libafl_nyx/src/settings.rs:69:5
   |
69 | /     pub fn with_timeout_micro_secs(mut self, timeout_micro_secs: u32) -> Self {
70 | |         self.timeout_micro_secs = timeout_micro_secs;
71 | |         self
72 | |     }
   | |_____^
   |
   = help: consider adding the `#[must_use]` attribute to the method or directly to the `Self` type
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#return_self_not_must_use

error: could not compile `libafl_nyx` (lib test) due to 16 previous errors

@l4yton l4yton marked this pull request as ready for review March 21, 2024 21:26
@tokatoka
Copy link
Member

thank you 👍

@tokatoka tokatoka merged commit 50843b1 into AFLplusplus:main Mar 21, 2024
27 checks passed
const DEFAULT_TIMEOUT_MICRO_SECS: u32 = 0;

#[derive(Debug, Clone, Copy, TypedBuilder)]
pub struct NyxSettings {
Copy link
Member

Choose a reason for hiding this comment

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

If anyone should feel like it, it could be nice to have some doc comments for these fields

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