-
Notifications
You must be signed in to change notification settings - Fork 147
Fixes to Host Call Fuzzing #840
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
Fixes to Host Call Fuzzing #840
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, lgtm
1. Setting sandbox snapshot to none in call_type_erased_guest_function_by_name 2. Restoring to an initial snapshot on each fuzzing iteration to avoid hitting a known memory leak. Signed-off-by: adamperlin <[email protected]>
6306e2d
to
426baaf
Compare
// Reset snapshot since we are mutating the sandbox state | ||
self.snapshot = None; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we clearing this here? Shouldn't the restore call above take care of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
otherwise I would have expected it too look like
pub fn call_guest_function_by_name<Output: SupportedReturnType>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a bug: since this function is wrapping call_guest_function_by_name_no_reset
and we're mutating the guest sandbox, the attached snapshot will be invalidated here, right? If we don't clear the snapshot, the restore call will hit the "snapshot exists" case and won't do the restore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the moment we could change the behavior to do the snapshot
and restore
in the call_type_erased_guest_function_by_name
though, but since it seemed more clear to do it in the fuzzing code!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I think I miss read this. I think this is fine and mimics the 'call func'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok, I'm glad it looks fine!
This PR prevents
fuzz_host_call
from hitting a memory leak upon encountering host function calling errors. It restores to a known clean snapshot on each iteration. This fix should not be needed once #826 is fixed.Snapshot restore initially wasn't working in the fuzzing case due to a bug discovered by @ludfjig in
call_type_erased_guest_function_by_name
(snapshot wasn't being set to None) so this bug has been fixed.This PR also more explicitly ignores some expected errors that may come up from host call fuzzing.