Fix brk() handling for snapshot module (#2970)

* drop grown address

* this clippy lint literally makes 0 sense!

* i hate you rust

* mm

* don't use drop! add comments for why alignment is not necessary
This commit is contained in:
Dongjia "toka" Zhang 2025-02-13 11:33:24 +01:00 committed by GitHub
parent bdcc0c56e4
commit 33db263260
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 31 additions and 15 deletions

View File

@ -162,6 +162,7 @@ module_name_repetitions = "allow"
unsafe_derive_deserialize = "allow" unsafe_derive_deserialize = "allow"
similar_names = "allow" similar_names = "allow"
too_many_lines = "allow" too_many_lines = "allow"
comparison_chain = "allow" # This lint makes **ZERO** sense
[workspace.lints.rustdoc] [workspace.lints.rustdoc]

View File

@ -407,11 +407,20 @@ impl SnapshotModule {
let aligned_new_brk = (new_brk + ((SNAPSHOT_PAGE_SIZE - 1) as GuestAddr)) let aligned_new_brk = (new_brk + ((SNAPSHOT_PAGE_SIZE - 1) as GuestAddr))
& (!(SNAPSHOT_PAGE_SIZE - 1) as GuestAddr); & (!(SNAPSHOT_PAGE_SIZE - 1) as GuestAddr);
log::debug!("New brk ({:#x?}) < snapshotted brk ({:#x?})! Mapping back in the target {:#x?} - {:#x?}", new_brk, self.brk, aligned_new_brk, aligned_new_brk + (self.brk - aligned_new_brk)); log::debug!("New brk ({:#x?}) < snapshotted brk ({:#x?})! Mapping back in the target {:#x?} - {:#x?}", new_brk, self.brk, aligned_new_brk, aligned_new_brk + (self.brk - aligned_new_brk));
drop(qemu.map_fixed( qemu.map_fixed(
aligned_new_brk, aligned_new_brk,
(self.brk - aligned_new_brk) as usize, (self.brk - aligned_new_brk) as usize,
MmapPerms::ReadWrite, MmapPerms::ReadWrite,
)); )
.unwrap();
} else if new_brk > self.brk {
// The heap has grown. so we want to drop those
let drop_sz = (new_brk - self.brk) as usize;
// if self.brk is not aligned this call will return an error
// and it will page align this drop_sz too
// look at target_munmap in qemu-libafl-bridge
qemu.unmap(self.brk, drop_sz).unwrap();
} }
for acc in &mut self.accesses { for acc in &mut self.accesses {
@ -426,11 +435,12 @@ impl SnapshotModule {
.query_mut(*page..(page + SNAPSHOT_PAGE_SIZE as GuestAddr)) .query_mut(*page..(page + SNAPSHOT_PAGE_SIZE as GuestAddr))
{ {
if !entry.value.perms.unwrap_or(MmapPerms::None).writable() { if !entry.value.perms.unwrap_or(MmapPerms::None).writable() {
drop(qemu.mprotect( qemu.mprotect(
entry.interval.start, entry.interval.start,
(entry.interval.end - entry.interval.start) as usize, (entry.interval.end - entry.interval.start) as usize,
MmapPerms::ReadWrite, MmapPerms::ReadWrite,
)); )
.unwrap();
entry.value.changed = true; entry.value.changed = true;
entry.value.perms = Some(MmapPerms::ReadWrite); entry.value.perms = Some(MmapPerms::ReadWrite);
} }
@ -464,11 +474,12 @@ impl SnapshotModule {
if !entry.value.perms.unwrap_or(MmapPerms::None).writable() if !entry.value.perms.unwrap_or(MmapPerms::None).writable()
&& !entry.value.changed && !entry.value.changed
{ {
drop(qemu.mprotect( qemu.mprotect(
entry.interval.start, entry.interval.start,
(entry.interval.end - entry.interval.start) as usize, (entry.interval.end - entry.interval.start) as usize,
MmapPerms::ReadWrite, MmapPerms::ReadWrite,
)); )
.unwrap();
entry.value.changed = true; entry.value.changed = true;
} }
} }
@ -487,11 +498,12 @@ impl SnapshotModule {
for entry in self.maps.tree.query_mut(0..GuestAddr::MAX) { for entry in self.maps.tree.query_mut(0..GuestAddr::MAX) {
if entry.value.changed { if entry.value.changed {
drop(qemu.mprotect( qemu.mprotect(
entry.interval.start, entry.interval.start,
(entry.interval.end - entry.interval.start) as usize, (entry.interval.end - entry.interval.start) as usize,
entry.value.perms.unwrap(), entry.value.perms.unwrap(),
)); )
.unwrap();
entry.value.changed = false; entry.value.changed = false;
} }
} }
@ -653,26 +665,29 @@ impl SnapshotModule {
if found.is_empty() { if found.is_empty() {
//panic!("A pre-snapshot memory region was unmapped"); //panic!("A pre-snapshot memory region was unmapped");
drop(qemu.map_fixed( qemu.map_fixed(
entry.interval.start, entry.interval.start,
(entry.interval.end - entry.interval.start) as usize, (entry.interval.end - entry.interval.start) as usize,
entry.value.perms.unwrap(), entry.value.perms.unwrap(),
)); )
.unwrap();
} else if found.len() == 1 && found[0].0 == *entry.interval { } else if found.len() == 1 && found[0].0 == *entry.interval {
if found[0].1 && found[0].2 != entry.value.perms { if found[0].1 && found[0].2 != entry.value.perms {
drop(qemu.mprotect( qemu.mprotect(
entry.interval.start, entry.interval.start,
(entry.interval.end - entry.interval.start) as usize, (entry.interval.end - entry.interval.start) as usize,
entry.value.perms.unwrap(), entry.value.perms.unwrap(),
)); )
.unwrap();
} }
} else { } else {
// TODO check for holes // TODO check for holes
drop(qemu.mprotect( qemu.mprotect(
entry.interval.start, entry.interval.start,
(entry.interval.end - entry.interval.start) as usize, (entry.interval.end - entry.interval.start) as usize,
entry.value.perms.unwrap(), entry.value.perms.unwrap(),
)); )
.unwrap();
} }
for (interval, ..) in found { for (interval, ..) in found {
@ -685,7 +700,7 @@ impl SnapshotModule {
to_unmap.push((*entry.interval, entry.value.changed, entry.value.perms)); to_unmap.push((*entry.interval, entry.value.changed, entry.value.perms));
} }
for (i, ..) in to_unmap { for (i, ..) in to_unmap {
drop(qemu.unmap(i.start, (i.end - i.start) as usize)); qemu.unmap(i.start, (i.end - i.start) as usize).unwrap();
new_maps.tree.delete(i); new_maps.tree.delete(i);
} }