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

hypercall: dump RANGE_SUBMIT filters in workdir #10

Closed
wants to merge 1 commit into from

Conversation

Wenzel
Copy link

@Wenzel Wenzel commented Jul 20, 2023

This PR aims to dump the PT filters set by the guest through RANGE_SUBMIT, in order for the fuzzer to parse them on shutdown, and update his own WORKDIR/config.yaml config dump.

The final goal is to enable users to launch kalf cov silently without further IP filters configuration required.

Related: IntelLabs/kafl.fuzzer#68

to be parsed by kAFL fuzzer on shutdown
@il-steffen
Copy link
Contributor

Maybe do this here, on first snapshot where PT is also setup for the rest of the campaign?

if (!setup_snapshot_once) {

Then it will work independently of how the range is configured (I think RANGE_SUBMIT will override previous host-side settings but not sure). Also, can make this a separate function "dump_pt_settings" or so..

@il-steffen
Copy link
Contributor

@schumilo any opinion on this?

@schumilo
Copy link

This sounds like a very front-end-specific task. Is there really a good reason to implement this in the backend instead of utilizing the existing feature set? Why not simply dump the guest-provided ranges via DUMP_FILE while also executing RANGE_SUBMIT (we could just put everything into a kAFL-guest specific helper function for that)? From there on, we can implement everything else in the front end (e.g., kafl-fuzz.py checks if that file exists; otherwise, we throw an error).

@il-steffen
Copy link
Contributor

There are at least two ways to configure this and only Qemu knows the true value at the time of setting up PT. I remember some discussion if RANGE_SUBMIT takes precedence over host-side settings or only overrides unused filter regions etc. Or what happens if you call it twice?

Doing this in the individual agents means that we cannot rely on this info to be there and accurate. We can look at it, and maybe it should take precedence over host side config settings or maybe not? It just continues the confusion..

We could report it back via the aux buffer and then the frontend could write it to the disk, but simply dumping the truth as seen by Qemu seems more reliable + useful feature.

@Wenzel
Copy link
Author

Wenzel commented Jul 20, 2023

There are at least two ways to configure this and only Qemu knows the true value at the time of setting up PT. I remember some discussion if RANGE_SUBMIT takes precedence over host-side settings or only overrides unused filter regions etc. Or what happens if you call it twice?

https://github.com/IntelLabs/kafl.qemu/blob/kafl_stable/nyx/hypercall/hypercall.c#L292

The hypercall checks whether the requested IP filter is already configured, and simply returns if that's the case.
Likewise, if you call it twice, the second call will be skipped since the IP filter has alreadu been configured before.

@schumilo
Copy link

Exactly! So, isn't it easier to implement this feature in the agent & front end (considering the extra effort to maintain, test and write a test only for this particular feature in the first place)?

In case the range was configured manually, the front end needs to create a file with the information for that in the workdir. If the RANGE_SUBMIT values are set by the guest and copied over to the host via DUMP_FILE, we will also get this information. And at that point, it's pretty straightforward for the front end to figure out which config file should be used. And in case neither the manual conf file nor the auto conf file exists, the front-end reports an error.

I won't argue if you still think this feature would be better implemented in the backend. But since this information is also stored in the snapshot, we could also try another approach and parse the actual snapshot files:

https://github.com/nyx-fuzz/QEMU-Nyx/blob/qemu-nyx-4.2.0/nyx/state/snapshot_state.h#L19

This is probably a much better approach than creating front-end-specific files based on the execution of specific hypercalls. However, currently, the snapshot is only created if we enable this feature (for instance, sometimes it simply makes no sense to dump and store the entire snapshot on the hard drive; tough that feature is always enabled in multiprocessing mode), but this is something we could quickly fix by always saving the metadata but not necessarily the entire snapshot data (like the RAM dump and block device diff). If parsing a binary file in Python is tricky, we could also use a custom format like JSON to store the information in a separate file.

@schumilo
Copy link

BTW: If the goal of this PR is that kafl cov is able to automatically configure the same PT ranges as used by the fuzzer, then why not just create a child process and load the snapshot? This way, everything is automatically configured just so and moreover we have the exact state as used and serialized by the parent process.

@il-steffen
Copy link
Contributor

kafl cov can already work based on the snapshot. The older code is still there (and overall a big mess now) but loading the snapshot is much more reliable especially since the pre-snapshot part is rarely used now. But if you then want to tell ptdump about the filter range, you still need to extract that info to the frontend somehow.

Directly creating coverage is also there, the fuzzer can save the binary pt dumps away during the fuzzing run and we decode them later. The only overhead is for qemu to actually write out the trace buffer and for the frontend to move (and zip it). But again, the later ptdump decoding needs to know the pt ranges.

So yes, we have the info in the frontend and we can write a file if done by the agent, it just seems to me that the ground truth is in qemu and that's where I would dump it from to have it always available and consistent. But hey, I'm not writing the patch... :-)

@schumilo
Copy link

Okay, so how about getting the trace region information from the snapshot files (workdir/snapshot) instead? Because the data is already there once the root snapshot is created and I would really prefer if we don't put the same functionality and features in different hypercalls to at least keep the code base somewhat neat and tidy. The only limitation I see is that the snapshot is not created if we don't set the -fast_vm_reload argument (not sure if this is an issue for kAFL, but libnyx uses that in standalone mode). In this case, the snapshot will be kept in memory and won't be serialized, but this is something we I can easily fix.

If this sounds reasonable to you, I could start with the PR next week. With that, I will fix the current limitation and if required extend the serializer to additionally store the meta-data (which includes information such as the configured trace regions) in a JSON-like format. My goal here is that both QEMU-Nyx repos remain to some degree aligned, but especially the current patch is something I would rather not like to merge into the main repo at a later point.

@schumilo
Copy link

How about the following PR instead? nyx-fuzz/QEMU-Nyx#53

@Wenzel
Copy link
Author

Wenzel commented Jul 27, 2023

Thanks for the discussion.
Even though I don't catch all the details and their ramifications, I'm in favor of a solution that doesn't imply to modify the existing kAFL agents or the expected hypercall protocol implemented by user (dumping an additional file) and creating an uncessary dependency between the frontend and the agent (frontend expects a certain config file to be dumped by the agent).

Since the information is already present in the root snapshot, let's reuse that.
Dumping an additional YAML metadata file sounds reasonable here.

What about the limitation of memory snapshots ? you said that it could be lifted quite easily.
should we switch to always writing the root snapshot to a file in the workdir ? (since we default on /dev/shm, it will be in memory anyway if I understand correctly)

@schumilo
Copy link

schumilo commented Aug 2, 2023

Seems like that this limitation does not affect kAFL at all; simply because it does not use the "skip_serialization" option:
https://github.com/IntelLabs/kafl.fuzzer/blob/master/kafl_fuzzer/worker/qemu.py#L139

However, all other libnyx-based fuzzers (such as AFL++) will require a libnyx update, but since we ship all components as submodules, we can fix that simply by updating all commit IDs accordingly.

The PR is now also ready:
nyx-fuzz/QEMU-Nyx#53

@schumilo
Copy link

schumilo commented Aug 2, 2023

The proposed patch is also already tested by me, but maybe I have missed something important. However, as soon as you have confirmed that everything works as expected with kAFL, I will merge this PR and also another one into the main branch (qemu-nyx-4.2.0). No idea if you plan to update the entire branch in your qemu repository or just cherry-pick the relevant patches; anyways, both PR are expected to work just fine with kAFL.

@Wenzel
Copy link
Author

Wenzel commented Aug 2, 2023

@schumilo I confirm your PR works fine with kAFL.
I updated the fuzzer patch a little bit to parse snapshot/state.yml instead
IntelLabs/kafl.fuzzer@9ec4709
image

LGTM !

@schumilo
Copy link

schumilo commented Aug 3, 2023

Okay, thanks for the update :-)
All patches are now merged into the main branch (qemu-nyx-4.2.0).

@Wenzel
Copy link
Author

Wenzel commented Aug 3, 2023

Closing this PR as nyx-fuzz/QEMU-Nyx#53 has been merged in upstream repo and integrated into latest v0.7 release

@Wenzel Wenzel closed this Aug 3, 2023
@Wenzel Wenzel deleted the hypercall/dump_range_submit branch August 3, 2023 15:19
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