Skip to content

Syx snapshot fixes and tuning #112

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

enisrat
Copy link

@enisrat enisrat commented Apr 8, 2025

  • Fix: CONFIG_SOFTMMU not available in blk
  • HMP commands for SYX testing & debugging
  • SYX in TLB streamlining
    • We should make proper use of QEMU`s dirty RAM functions
    • Integrate with notdirty_write, this is essentially the same as what we want
  • Add libafl_blk_write to write to blkdev with SYX enabled

return flags;
}

//// --- Begin LibAFL code ---
// Use this snippet multiple times below
#define SYX_SNAPSHOT_DIRTY_LIST_ADD_HOSTADDR_PROBE(dbg, access_type, addr, entry_full, phost) { \
Copy link
Member

Choose a reason for hiding this comment

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

this should be moved to top of this file

@@ -29,9 +29,8 @@
#include "migration/misc.h"

//// --- Begin LibAFL code ---
#ifdef CONFIG_SOFTMMU
//#define CONFIG_DEBUG_SYX
Copy link
Member

Choose a reason for hiding this comment

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

clean this

@@ -16,6 +16,7 @@
#include "device-save.h"
#include "syx-cow-cache.h"

//#define CONFIG_DEBUG_SYX
Copy link
Member

Choose a reason for hiding this comment

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

clean up this

*(int *)opaque = ret;
}

int libafl_blk_write(BlockBackend *blk, void *buf, int64_t offset, int64_t sz)
Copy link
Member

Choose a reason for hiding this comment

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

why do we need this? i don't see it used anywhere else in your pr.

Copy link
Author

Choose a reason for hiding this comment

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

Ah yes you are right, it is not related directly to SYX.

I placed this in system.h.
I use it a lot in my branches for writing the fuzzing input to blockdev. The write is catched by SYX.
Often in embedded firmwares, fuzzing blocks of Flash or EMMC (firmware headers, partition tables, structures, etc.)
Think of it as cpu_physical_memory_rw for blockdev.

This is only low-level api yet, I can remove it of course.

@@ -227,9 +227,12 @@ void syx_cow_cache_read_entry(SyxCowCache* scc, BlockBackend* blk,
if (!QTAILQ_EMPTY(&scc->layers)) {
for (; qiov_offset < qiov->size;
blk_offset += chunk_size, qiov_offset += chunk_size) {
int i = 0;
Copy link
Member

Choose a reason for hiding this comment

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

clean up this

QTAILQ_FOREACH(layer, &scc->layers, next)
{
chunk_size = layer->chunk_size;
//printf("[SYX] check cache layer %d\n", i);
Copy link
Member

Choose a reason for hiding this comment

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

clean up this

@@ -217,8 +218,7 @@ void syx_cow_cache_read_entry(SyxCowCache* scc, BlockBackend* blk,
size_t qiov_offset = 0;
uint64_t chunk_size = 0;

// printf("[%s] Read 0x%zx bytes @addr %lx\n", blk_name(blk), qiov->size,
// offset);
//printf("[SYX] [%s] Read 0x%zx bytes @addr %lx\n", blk_name(blk), qiov->size, offset);
Copy link
Member

Choose a reason for hiding this comment

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

clean up this

@@ -0,0 +1,34 @@
#include "libafl/syx-snapshot/syx-snapshot.h"
Copy link
Member

Choose a reason for hiding this comment

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

it this useful? for full snapshots i'd use qemu snapshots instead, which are already available with hmp.
may be useful for debugging maybe?

Copy link
Author

Choose a reason for hiding this comment

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

It is for debugging only. I used it to resolve the issues fixed in this PR. I have a personal branch with more features and also many more HMP commands.
As SYX (still don't know what that stands for ;-) ) is quite complicated I cannot debug it via the fuzzer lib. I usually compile the brigde stand-alone and do manual tests.

@@ -1406,16 +1412,19 @@ static int probe_access_internal(CPUState *cpu, vaddr addr,

/* Everything else is RAM. */
*phost = (void *)((uintptr_t)addr + entry->addend);
//// --- Begin LibAFL code ---
Copy link
Member

Choose a reason for hiding this comment

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

why move this to higher-level functions?

i start to think we should not keep this call in probe, there are many places where memory gets probed for write but it in fact never written to.

it's only a matter of optimization though, nothing too problematic.

Copy link
Author

Choose a reason for hiding this comment

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

The probes are used by guest helpers. Even though not every probe may lead to an actual write, we have no way to get this feedback as of now. This is how the QEMU TCG is set up.

A rule of thumb for me was the following:
Whenever QEMU uses notdirty_write, we want to make a backup of the page (syx_snapshot_dirty_list_add_hostaddr). Because these are mostly the same features.

But we cannot use notdirty_write, it misses hostaddr, so I came up with this.

@enisrat
Copy link
Author

enisrat commented May 17, 2025

Hi, thanks for the review!

I did some cleanup.

Also, please note that I removed CONFIG_SOFTMMU from block-backend.c because CONFIG_SOFTMMU is not available in block_ss.
This means that before this SYX features was not even compiled.
Now the QEMU block tools and tests will not compile anymore, so we have to use --disable-tools --disable-tests. If that is a problem we could address it in a future PR.

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.

2 participants