make afl-system-config available as system-config

[vanhauser-thc]

No description provided.

[smoelius]

Thank you very much for this PR.

However, I'm not sure how I feel about this:

afl.rs/afl/src/bin/cargo-afl.rs

Line 175 in 878870e

.about("Invoke afl-system-config (hint: use with sudo)")

In particular, I'm not sure we should suggest that users run cargo <anything> under sudo. cargo is a big code base, and there are lots of places where demons could lurk.

Something like this came up before, and the solution I chose then was to instruct the user on how to run sudo afl-system-config directly:

afl.rs/afl/src/bin/cargo-afl.rs

Lines 233 to 242 in 8c8a0cd

	let sudo_cmd_path = common::afl_dir(None).join("bin").join("afl-system-config");
	eprintln!(
	"
	If you see an error message like `shmget() failed` above, try running the following command:
	sudo {}
	Note: You will be prompted to enter your password.",
	sudo_cmd_path.display()
	);

Do you sympathize with my concerns about sudo cargo ...? If so, are there other ways we might make running afl-system-config easier?

[vanhauser-thc]

I am not a big rust guy :)
now that you point it out, "sudo cargo run afl system-config" could result in a lot of files being created by root that will make future compilations fails if done without sudo.

IMHO cargo run afl system-config is important to have, but it should be done different.
how about running it "sudo afl-system-config" and bail if that fails with an error that helps what should be done?

[vanhauser-thc]

how about this?

[smoelius]

how about running it "sudo afl-system-config" and bail if that fails with an error that helps what should be done?

Just to be sure we're on the same page, you're suggesting that the afl-system-config subcommand be "special":

• All of the other subcommands continue to run an afl++ binary or Cargo subcommand directly.
• But rather than run <path-to-afl-system-config>, the afl-system-config subcommand runs sudo <path-to-afl-system-config>.

Is that right?

[vanhauser-thc]

Yes exactly

[smoelius]

Thank you again for this PR. I agree that having a system-config subcommand will be convenient.

[vanhauser-thc]

I implemented all changes with the exception to make the sudo call stuff prettier/streamlined. this is beyond my limited rust skills ... :(

[smoelius]

I'm trying to understand the CI failures, which has caused me to find this explanation of LaunchAgents vs. LaunchDaemons: https://ss64.com/osx/launchctl.html

Furthermore, I see these lines in afl-system-config (note the use of sudo on the second line):

    launchctl unload -w ${SL}/LaunchAgents/${PL}.plist >/dev/null 2>&1
    sudo launchctl unload -w ${SL}/LaunchDaemons/${PL}.Root.plist >/dev/null 2>&1

That is, for Mac, it looks like afl-system-config is expected to be run not under sudo.

Am I misinterpreting the above lines?

[smoelius]

FWIW, here is a slight reworking of afl-system-config that expects to be called with sudo: AFLplusplus/AFLplusplus@stable...smoelius:AFLplusplus:stable

With this variant, CI passes.

How do you think we should proceed?

[vanhauser-thc]

That is, for Mac, it looks like afl-system-config is expected to be run not under sudo.

no it needs to run under root/run with sudo. I do not know why there is an extra sudo in there :-) I did not code that part. but it works :)

I merged your changes to afl-system-config, I booted up my macos mini I never use and it didnt work before, with your patch it does.

so this will need a new afl++ release or are you fine to pin AFL++ to the current stable branch?

[smoelius]

I merged your changes to afl-system-config, I booted up my macos mini I never use and it didnt work before, with your patch it does.

I'm glad to hear that!

so this will need a new afl++ release or are you fine to pin AFL++ to the current stable branch?

I'm fine to pin to the current stable branch for now.

I would like to treat this new subcommand as a breaking change. There are other changes I want to include in the next breaking-change release (e..g., this and this), and it will take some time for me to prepare them.

If a new AFL++ release is not out before then, we can revisit.

Can I impose on you to commit the last two suggestions and update the AFL++ submodule? After that, I think this should be mergeable.

[vanhauser-thc]

Made the last changes, update the AFL++ commit, CI is now green.
IHMO it is not a breaking change. It adds a new command, so everyone can just work as before and ignore it :)
but up to you obviously.

[smoelius]

IHMO it is not a breaking change. It adds a new command, so everyone can just work as before and ignore it :)

You're right, of course. My reason for wanting to consider the change breaking is that if (hypothetically) a user objects to cargo-afl calling sudo, they can stick with version 0.13.

Thanks a lot for all of your hard work on this.