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

fuzzing: initial fuzzing implementation #113

Merged
merged 7 commits into from
Oct 23, 2023
Merged

Conversation

00xc
Copy link
Member

@00xc 00xc commented Oct 2, 2023

This PR adds:

  • Basic fuzzing infrastructure via cargo-fuzz.
  • Harnesses for the following interfaces:
    • Firmware metadata (fw_meta)
    • ACPI table parsing.
    • Filesystem operations.
  • Relevant documentation.
  • Updates to CI to run on the new pieces of code.

Works towards #34

Copy link
Member

@joergroedel joergroedel left a comment

Choose a reason for hiding this comment

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

I made a few comments, but besides those this looks good overall. Thanks for working on fuzzing, it already uncovered some bugs.

FUZZING.md Show resolved Hide resolved
src/acpi/tables.rs Outdated Show resolved Hide resolved
src/acpi/tables.rs Outdated Show resolved Hide resolved
src/acpi/tables.rs Outdated Show resolved Hide resolved
src/fw_cfg.rs Outdated Show resolved Hide resolved
@00xc 00xc force-pushed the fuzzing branch 4 times, most recently from 56abe65 to e8f0cba Compare October 14, 2023 11:35
@00xc
Copy link
Member Author

00xc commented Oct 14, 2023

Also made parse_fw_meta_data() unsafe, as the caller must guarantee that the provided VirtAddr is valid and points to a region of the adequate size (4k).

@00xc 00xc force-pushed the fuzzing branch 3 times, most recently from 1740cb1 to 4ebf471 Compare October 17, 2023 20:36
@00xc
Copy link
Member Author

00xc commented Oct 18, 2023

Last version: rebased on main and fixed the ACPI harness to accommodate the new Sync bound for the IOPort trait.

fuzz/fuzz_targets/fs.rs Outdated Show resolved Hide resolved
fuzz/fuzz_targets/fs.rs Outdated Show resolved Hide resolved
fuzz/fuzz_targets/fs.rs Show resolved Hide resolved
src/fw_meta.rs Outdated Show resolved Hide resolved
00xc added 2 commits October 18, 2023 22:18
Refactor the public function parse_fw_meta_data() so that the caller
can provide an slice containing the firmware metadata. This will help
building a fuzzing harness around this interface

Signed-off-by: Carlos López <[email protected]>
Add fuzzing to the COCONUT SVSM project via cargo-fuzz. This commit
adds the base infrastructure for fuzzing, as well as two harnesses
for testing the fw_meta and ACPI table interfaces respectively.

This works towards issue coconut-svsm#34.

Signed-off-by: Carlos López <[email protected]>
Copy link
Contributor

@Freax13 Freax13 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 to me!

src/fw_meta.rs Show resolved Hide resolved
00xc added 5 commits October 20, 2023 14:28
Add documentation (FUZZING.md) regarding the use of fuzzing harnesses
via cargo-fuzz.

Signed-off-by: Carlos López <[email protected]>
We were runninng clippy on tests in Github Actions, but not locally.
Additionally, run it con fuzzing harnesses as well.

Signed-off-by: Carlos López <[email protected]>
Add a new method to retrieve the current position of the FileHandle.

Signed-off-by: Carlos López <[email protected]>
Add a new harness to fuzz the filesystem implementation by issuing
random operations such as creating, opening and closing files and
reading and writing from them.

Signed-off-by: Carlos López <[email protected]>
00xc added a commit to 00xc/svsm that referenced this pull request Oct 20, 2023
Limit hypervisor-controlled allocation sizes to prevent potential OOM
issues. This is done in places where big allocation sizes(for example
a huge number of firmware files or very big ACPI tables) would make
no sense in a reasonable implementation.

This will also be useful for fuzzing performance once coconut-svsm#113 is merged.

Signed-off-by: Carlos López <[email protected]>
@00xc
Copy link
Member Author

00xc commented Oct 20, 2023

Dropped fuzzing: limit hypervisor-controlled allocation sizes in favor of #140. With that this PR has no unrelated commits.

@00xc 00xc requested a review from joergroedel October 20, 2023 12:36
Copy link
Member

@joergroedel joergroedel 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 now.

@joergroedel joergroedel merged commit 47a1af3 into coconut-svsm:main Oct 23, 2023
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