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.

Sign up for GitHub

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

Add transitional message #348

Merged
merged 2 commits into from
Jun 28, 2023
Merged

Add transitional message #348

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
06f8195
Add transitional message
smoelius Jun 27, 2023
8128d20
Mention that --force may be needed
smoelius Jun 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .github/workflows/rust.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ jobs:
fail-fast: false
matrix:
environment: [ubuntu-latest, macos-latest]
package: [afl, cargo-afl]
toolchain: [stable, nightly]
cc: [cc, clang]
include:
Expand Down Expand Up @@ -65,9 +66,9 @@ jobs:
sudo launchctl unload -w ${SL}/LaunchDaemons/${PL}.Root.plist
fi
- name: Build examples (with AFL instrumentation)
run: cargo run afl build --examples -vv
run: cargo run -p ${{ matrix.package }} -- afl build --examples -vv
- name: Run tests (with AFL instrumentation)
run: cargo run afl test -vv
run: cargo run -p ${{ matrix.package }} -- afl test -vv
all-checks:
needs: [lint, build]
runs-on: ubuntu-latest
Expand Down
68 changes: 68 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 6 additions & 34 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,6 @@
[package]
name = "afl"
version = "0.13.1"
readme = "README.md"
license = "Apache-2.0"
authors = ["Keegan McAllister <[email protected]>",
"Corey Farwell <[email protected]>"]
description = "Fuzzing Rust code with american-fuzzy-lop"
repository = "https://github.com/rust-fuzz/afl.rs"
homepage = "https://github.com/rust-fuzz/afl.rs"
edition = "2021"

[build-dependencies]
fs_extra = "1.3"
home = "0.5"
rustc_version = "0.4"
tempfile = "3.6"
xdg = "2.4"

[dependencies]
clap = { version = "4.3", features = ["cargo"] }
lazy_static = { version = "1.4.0", optional = true }
libc = "0.2.147"
rustc_version = "0.4"
xdg = "2.4"

[dev-dependencies]
arbitrary = { version = "1", features = ["derive"] }
assert_cmd = "2.0"
tempfile = "3.6"

[features]
reset_lazy_static = ["lazy_static"]
no_cfg_fuzzing = []
[workspace]
members = [
"afl",
"cargo-afl",
]
resolver = "2"
35 changes: 35 additions & 0 deletions afl/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
[package]
name = "afl"
version = "0.13.1"
readme = "../README.md"
license = "Apache-2.0"
authors = ["Keegan McAllister <[email protected]>",
"Corey Farwell <[email protected]>"]
description = "Fuzzing Rust code with american-fuzzy-lop"
repository = "https://github.com/rust-fuzz/afl.rs"
homepage = "https://github.com/rust-fuzz/afl.rs"
edition = "2021"

[build-dependencies]
fs_extra = "1.3"
home = "0.5"
libc = "0.2"
rustc_version = "0.4"
tempfile = "3.6"
xdg = "2.4"

[dependencies]
clap = { version = "4.3", features = ["cargo"] }
lazy_static = { version = "1.4", optional = true }
libc = "0.2"
rustc_version = "0.4"
xdg = "2.4"

[dev-dependencies]
arbitrary = { version = "1", features = ["derive"] }
assert_cmd = "2.0"
tempfile = "3.6"

[features]
reset_lazy_static = ["lazy_static"]
no_cfg_fuzzing = []
56 changes: 52 additions & 4 deletions build.rs → afl/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::env;
use std::path::{Path, PathBuf};
use std::process::Command;

static AFL_SRC_PATH: &str = "AFLplusplus";
static AFL_SRC_PATH: &str = "../AFLplusplus";

// https://github.com/rust-fuzz/afl.rs/issues/148
#[cfg(target_os = "macos")]
Expand All @@ -16,11 +16,25 @@ mod common;
fn main() {
let installing = home::cargo_home()
.map(|path| Path::new(env!("CARGO_MANIFEST_DIR")).starts_with(path))
.unwrap();
.unwrap()
|| env::var("TESTING_INSTALL").is_ok();

let building_cargo_afl = env::var("CARGO_PKG_NAME") == Ok(String::from("cargo-afl"));

let building_on_docs_rs = env::var("DOCS_RS").is_ok();

let out_dir = env::var("OUT_DIR").unwrap();

if installing && !building_cargo_afl {
println!("cargo:warning=You appear to be installing `cargo-afl` with:");
println!("cargo:warning= cargo install afl");
println!("cargo:warning=A future version of afl.rs will require you to use:");
println!("cargo:warning= cargo install cargo-afl");
println!("cargo:warning=You can use the new command now, if you like.");
}
Copy link

Choose a reason for hiding this comment

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

Does it make sense to also suggest cargo uninstall afl, or is the latter cargo-afl going to effectively clobber the install anyways?

Copy link
Member Author

Choose a reason for hiding this comment

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

Rather than suggest to uninstall, I propose the following. Change the first line to:

warning: You appear to be installing the `cargo-afl` binary with:

And add the following line at the end:

warning: Note: If the binary is already installed, you may need to add --force.

So the revised message would look like:

warning: You appear to be installing the `cargo-afl` binary with:
warning:     cargo install afl
warning: A future version of afl.rs will require you to use:
warning:     cargo install cargo-afl
warning: You can use the new command now, if you like.
warning: Note: If the binary is already installed, you may need to add --force.

What do you think?

Copy link

Choose a reason for hiding this comment

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

Sounds good to me!


// smoelius: Build AFLplusplus in a temporary directory when installing or when building on docs.rs.
let work_dir = if installing || env::var("DOCS_RS").is_ok() {
let work_dir = if installing || building_on_docs_rs {
let tempdir = tempfile::tempdir_in(&out_dir).unwrap();
if Path::new(AFL_SRC_PATH).join(".git").is_dir() {
let status = Command::new("git")
Expand All @@ -44,12 +58,16 @@ fn main() {
PathBuf::from(AFL_SRC_PATH)
};

let base = if env::var("DOCS_RS").is_ok() {
let base = if building_on_docs_rs {
Some(PathBuf::from(out_dir))
} else {
None
};

// smoelius: Lock `work_dir` until the build script exits.
#[cfg(unix)]
let _file = sys::lock_path(&work_dir).unwrap();
Comment on lines +71 to +72
Copy link

Choose a reason for hiding this comment

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

Just making sure I understand: this doesn't require an associated flock(..., LOCK_UN) because you're expecting the advisory lock to be dropped when the underlying fd is closed at process exit, right?

I believe that should be fine, but it's not super well defined by POSIX AFAICT. Just something to be aware of should it cause unusual behavior on non-Linux builds in the future 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

Just making sure I understand: this doesn't require an associated flock(..., LOCK_UN) because you're expecting the advisory lock to be dropped when the underlying fd is closed at process exit, right?

That's correct.

I believe that should be fine, but it's not super well defined by POSIX AFAICT. Just something to be aware of should it cause unusual behavior on non-Linux builds in the future 🙂

I'm curious as to what makes you say that (not saying you're wrong).

I'm reading this: https://man7.org/linux/man-pages/man2/flock.2.html

Furthermore, the lock is released
either by an explicit LOCK_UN operation on any of these duplicate
file descriptors, or when all such file descriptors have been
closed.

Is file descriptor closure not well defined?

Copy link

Choose a reason for hiding this comment

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

Is file descriptor closure not well defined?

I thought POSIX was ambiguous on whether process termination is actually required to close all FDs, but it looks like it's unambiguous:

All of the file descriptors, directory streams, conversion descriptors, and message catalog descriptors open in the calling process shall be closed.

(From https://pubs.opengroup.org/onlinepubs/007904875/functions/exit.html)

There's some subtlety around child processes that inherit the open FD, but I don't think that case matters here. So this is safe to ignore 🙂


build_afl(&work_dir, base.as_deref());
build_afl_llvm_runtime(&work_dir, base.as_deref());
}
Expand Down Expand Up @@ -85,3 +103,33 @@ fn build_afl_llvm_runtime(work_dir: &Path, base: Option<&Path>) {
.expect("could not run 'ar'");
assert!(status.success());
}

#[cfg(unix)]
mod sys {
use std::fs::File;
use std::io::{Error, Result};
use std::os::unix::io::AsRawFd;
use std::path::Path;

pub fn lock_path(path: &Path) -> Result<File> {
let file = File::open(path)?;
lock_exclusive(&file)?;
Ok(file)
}

// smoelius: `lock_exclusive` and `flock` were copied from:
// https://github.com/rust-lang/cargo/blob/ae91d4ed41da98bdfa16041dbc6cd30287920120/src/cargo/util/flock.rs

fn lock_exclusive(file: &File) -> Result<()> {
flock(file, libc::LOCK_EX)
}

fn flock(file: &File, flag: libc::c_int) -> Result<()> {
let ret = unsafe { libc::flock(file.as_raw_fd(), flag) };
if ret < 0 {
Err(Error::last_os_error())
} else {
Ok(())
}
}
}
0 examples/arbitrary.rs → afl/examples/arbitrary.rs
View file
Open in desktop
File renamed without changes.
0 examples/cmplog.rs → afl/examples/cmplog.rs
View file
Open in desktop
File renamed without changes.
0 examples/hello.rs → afl/examples/hello.rs
View file
Open in desktop
File renamed without changes.
0 input/zero → afl/input/zero
View file
Open in desktop
File renamed without changes.
0 src/bin/cargo-afl.rs → afl/src/bin/cargo-afl.rs
View file
Open in desktop
File renamed without changes.
0 src/common.rs → afl/src/common.rs
View file
Open in desktop
File renamed without changes.
0 src/lib.rs → afl/src/lib.rs
View file
Open in desktop
File renamed without changes.
37 changes: 37 additions & 0 deletions cargo-afl/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
[package]
name = "cargo-afl"
version = "0.13.1"
readme = "../README.md"
license = "Apache-2.0"
authors = ["Keegan McAllister <[email protected]>",
"Corey Farwell <[email protected]>",
"Samuel Moelius <[email protected]>"]
description = "Fuzzing Rust code with american-fuzzy-lop"
repository = "https://github.com/rust-fuzz/afl.rs"
homepage = "https://github.com/rust-fuzz/afl.rs"
edition = "2021"

build = "../afl/build.rs"

[[bin]]
name = "cargo-afl"
path = "../afl/src/bin/cargo-afl.rs"

[build-dependencies]
fs_extra = "1.3"
home = "0.5"
libc = "0.2"
rustc_version = "0.4"
tempfile = "3.6"
xdg = "2.4"

[dependencies]
clap = { version = "4.3", features = ["cargo"] }
libc = "0.2"
rustc_version = "0.4"
xdg = "2.4"

[dev-dependencies]
assert_cmd = "2.0"
predicates = "3.0"
tempfile = "3.6"
Loading