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

rmm: Initial fuzzing setup #418

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

Conversation

Skryptonyte
Copy link

(LFX Task 1)

This PR adds the initial fuzzing setup based on libfuzzer-sys along with two harnesses for RMI commands: FEATURES and VERSION.

Two scripts are provided:

  1. ./scripts/fuzz.sh <fuzzer_name> <var_args> - Runs the fuzzer. The script will run the fuzzer natively if the host is aarch64, otherwise it will fall back to QEMU userspace emulator.
  2. ./scripts/fuzz-coverage.sh <fuzzer_name> <duration> - Based on ./scripts/code-coverage.sh, the fuzzer is run for a set duration and an html report is generated after at ./code-coverage/index.html.

@Skryptonyte Skryptonyte force-pushed the lfx_fuzz branch 3 times, most recently from 5ebd870 to 31be3c3 Compare February 9, 2025 23:28
@Skryptonyte Skryptonyte marked this pull request as ready for review February 14, 2025 11:41
@Skryptonyte
Copy link
Author

Just rebased. I think the CI should work now.

@zpzigi754
Copy link
Collaborator

zpzigi754 commented Feb 18, 2025

CI log says that cargo check --workspace --exclude islet_sdk --exclude ciborium --exclude islet_mc_harnesses has failed with the included changes. As the error is related to std (error[E0463]: can't find crate for std), I guess that it might be related to the inclusion of an unexpected dependency.

I attach a part of the log below.

warning: `vmsa` (lib) generated 3 warnings
    Checking spin v0.9.8
    Checking tinyvec v1.6.0
    Checking log v0.4.21
   Compiling libfuzzer-sys v0.4.8
    Checking hex v0.4.3
    Checking arbitrary v1.4.1
error[E0463]: can't find crate for `std`
  |
  = note: the `aarch64-unknown-none-softfloat` target may not support the standard library
  = note: `std` is required by `arbitrary` because it does not declare `#![no_std]`
  = help: consider building the standard library from source with `cargo build -Zbuild-std`

    Checking uart v0.0.1 (/home/runner/work/islet/islet/lib/uart)
    Checking bitflags v1.3.2
error[E0463]: can't find crate for `std`
 --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/arbitrary-1.4.1/src/error.rs:1:5
  |
1 | use std::{error, fmt};
  |     ^^^ can't find crate
  |
  = note: the `aarch64-unknown-none-softfloat` target may not support the standard library
  = help: consider building the standard library from source with `cargo build -Zbuild-std`

@zpzigi754
Copy link
Collaborator

I've fixed the issue with the pushed commit.

timer.cntp_cval_el0 = CNTP_CVAL_EL0.get();
timer.cntp_ctl_el0 = CNTP_CTL_EL0.get();
timer.cnthctl_el2 = CNTHCTL_EL2.get();
#[cfg(not(fuzzing))]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than putting it here, could you handle it at into/from_current() in rmm/src/rec/context.rs? This looks more cosistent to me.

Copy link
Author

Choose a reason for hiding this comment

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

I had attempted this, but the compiler would still complain about the following.

error: expected readable system register
  --> /home/parallels/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aarch64-cpu-10.0.0/src/registers/macros.rs:18:42
   |
18 | ...   core::arch::asm!(concat!($asm_instr, " {reg:", $asm_width, "}, ", ...
   |                        ^
   |
note: instantiated into assembly here
  --> <inline asm>:1:10
   |
1  |     mrs x8, CNTPOFF_EL2
   |             ^

error: expected writable system register or pstate
  --> /home/parallels/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aarch64-cpu-10.0.0/src/registers/macros.rs:40:42
   |
40 | ...   core::arch::asm!(concat!($asm_instr, " ", $asm_reg_name, ", {reg:"...
   |                        ^
   |
note: instantiated into assembly here
  --> <inline asm>:1:6
   |
1  |     msr CNTPOFF_EL2, x1

I followed your approach but also added the cfg switch to the entire save_state and restore_state functions in timer.rs to make it compile successfully. Let me know if this is acceptable to you.

@@ -0,0 +1,26 @@
[package]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Refer other manifest file format, for instance, islet/rmm/Cargo.toml, and fill the fields on [package].

Copy link
Author

Choose a reason for hiding this comment

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

Done, I fixed up the author name and versioning to be consistent with other crate's manifests.

Skryptonyte and others added 4 commits February 19, 2025 18:52
It may resolve the below error in CI's rule-checker.
```
error[E0463]: can't find crate for `std`
  |
  = note: the `aarch64-unknown-none-softfloat` target may not support the standard library
  = note: `std` is required by `arbitrary` because it does not declare `#![no_std]`
  = help: consider building the standard library from source with `cargo build -Zbuild-std`

    Checking uart v0.0.1 (/home/runner/work/islet/islet/lib/uart)
    Checking bitflags v1.3.2
```

Signed-off-by: Changho Choi <[email protected]>
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.

4 participants