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

Address comments from #2302 #2322

Merged
merged 6 commits into from
Jun 18, 2024
Merged

Address comments from #2302 #2322

merged 6 commits into from
Jun 18, 2024

Conversation

domenukk
Copy link
Member

A bunch of smaller cleanups addressing my comments in PR #2302


// # Safety
// The `multi_machine_receiver_hook` needs messages to outlive the receiver.
// The underlying memory region for incoming messages lives longer than the async thread processing them.
Copy link
Member Author

Choose a reason for hiding this comment

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

@rmalmain Are we sure this is the case? Is there any race condition when memory gets cleared up before the async thread kicks in?

Copy link
Collaborator

Choose a reason for hiding this comment

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

afaik the messages live long enough. i'll double check this

@domenukk
Copy link
Member Author

@rmalmain can you take a look if you're happy with these changes?

@rmalmain
Copy link
Collaborator

@domenukk looks good to me, thanks for cleaning up

// We start ourself as child process to actually fuzz
let (staterestorer, _new_shmem_provider, core_id) = if env::var(_ENV_FUZZER_SENDER).is_err()
{
let broker_things = |mut broker: TcpEventBroker<S::Input, MT>, _remote_broker_addr| {
Copy link
Member

Choose a reason for hiding this comment

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

this is not really working for remote host...
this is the only place that remote_broker_addr is every passed, and it completely ignores the address

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes would be good to fix it at some point

@domenukk domenukk merged commit e64f0fb into main Jun 18, 2024
98 of 100 checks passed
@domenukk domenukk deleted the tree_cleanup branch June 18, 2024 13:58
R9295 pushed a commit to R9295/LibAFL that referenced this pull request Jun 21, 2024
* Address comments from AFLplusplus#2302

* secure?

* cleanup

* early exit ftw

* address clippy

* Fix all the things
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