Skip to content

Commit d3260d5

Browse files
koutekoute
committed
Fix write_memory aux data access check on the recompiler; add set_host_side_aux_write_protect 
1 parent db1df41 commit d3260d5

File tree

4 files changed

+124
-14
lines changed

4 files changed

+124
-14
lines changed

‎crates/polkavm/src/api.rs‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,7 @@ impl Module {
746746
module: self.clone(),
747747
backend,
748748
crosscheck_instance,
749+
host_side_aux_write_protect: false,
749750
})
750751
}
751752

@@ -1033,6 +1034,7 @@ pub struct RawInstance {
10331034
module: Module,
10341035
backend: InstanceBackend,
10351036
crosscheck_instance: Option<Box<InterpretedInstance>>,
1037+
host_side_aux_write_protect: bool,
10361038
}
10371039

10381040
impl RawInstance {
@@ -1259,7 +1261,25 @@ impl RawInstance {
12591261

12601262
access_backend!(self.backend, |mut backend| backend
12611263
.set_accessible_aux_size(size)
1262-
.into_result("failed to set accessible aux size"))
1264+
.into_result("failed to set accessible aux size"))?;
1265+
1266+
debug_assert_eq!(access_backend!(self.backend, |backend| backend.accessible_aux_size()), size);
1267+
Ok(())
1268+
}
1269+
1270+
/// Sets whether the aux data region is write-protected on the host-side.
1271+
///
1272+
/// This affects `write_memory`, `zero_memory` and `is_memory_accessible`.
1273+
///
1274+
/// Will return an error if called on an instance with dynamic paging enabled.
1275+
/// Infallible otherwise.
1276+
pub fn set_host_side_aux_write_protect(&mut self, is_write_protected: bool) -> Result<(), Error> {
1277+
if self.module.is_dynamic_paging() {
1278+
return Err("write-protecting the aux data region is only possible on modules without dynamic paging".into());
1279+
}
1280+
1281+
self.host_side_aux_write_protect = is_write_protected;
1282+
Ok(())
12631283
}
12641284

12651285
/// Resets the VM's memory to its initial state.
@@ -1285,7 +1305,8 @@ impl RawInstance {
12851305
return false;
12861306
}
12871307

1288-
if u64::from(address) + cast(size).to_u64() > 0x100000000 {
1308+
let upper_limit = if is_writable { self.get_write_upper_limit() } else { 0x100000000 };
1309+
if u64::from(address) + cast(size).to_u64() > upper_limit {
12891310
return false;
12901311
}
12911312

@@ -1391,6 +1412,15 @@ impl RawInstance {
13911412
result
13921413
}
13931414

1415+
fn get_write_upper_limit(&self) -> u64 {
1416+
if self.host_side_aux_write_protect {
1417+
debug_assert!(!self.module.is_dynamic_paging());
1418+
u64::from(self.module.memory_map().stack_address_high())
1419+
} else {
1420+
0x100000000
1421+
}
1422+
}
1423+
13941424
/// Writes into the VM's memory.
13951425
///
13961426
/// When dynamic paging is enabled calling this can be used to resolve a segfault. It can also
@@ -1407,7 +1437,7 @@ impl RawInstance {
14071437
});
14081438
}
14091439

1410-
if u64::from(address) + cast(data.len()).to_u64() > 0x100000000 {
1440+
if u64::from(address) + cast(data.len()).to_u64() > self.get_write_upper_limit() {
14111441
return Err(MemoryAccessError::OutOfRangeAccess {
14121442
address,
14131443
length: cast(data.len()).to_u64(),
@@ -1551,7 +1581,7 @@ impl RawInstance {
15511581
});
15521582
}
15531583

1554-
if u64::from(address) + u64::from(length) > 0x100000000 {
1584+
if u64::from(address) + u64::from(length) > self.get_write_upper_limit() {
15551585
return Err(MemoryAccessError::OutOfRangeAccess {
15561586
address,
15571587
length: u64::from(length),

‎crates/polkavm/src/sandbox/generic.rs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -931,9 +931,6 @@ impl Sandbox {
931931
};
932932

933933
if address >= self.aux_data_address && address_end < self.aux_data_address + self.aux_data_full_length {
934-
if is_writable {
935-
return Ok(());
936-
}
937934
if address_end > self.aux_data_address + self.aux_data_length {
938935
return Err(());
939936
}

‎crates/polkavm/src/sandbox/linux.rs‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1995,11 +1995,10 @@ impl super::Sandbox for Sandbox {
19951995
if !self.dynamic_paging_enabled {
19961996
let memory_map = module.memory_map();
19971997
let is_ok = if address >= memory_map.aux_data_address() {
1998-
let aux_data_size = module.memory_map().aux_data_size();
1999-
let aux_data_end = module.memory_map().aux_data_address() + aux_data_size;
2000-
let address_end = address as usize + data.len();
2001-
if address_end <= aux_data_end as usize {
2002-
self.memory_mmap.as_slice_mut()[address as usize..address as usize + data.len()].copy_from_slice(data);
1998+
let aux_data_end = module.memory_map().aux_data_address() + self.aux_data_length;
1999+
let address_end = cast(address).to_usize() + data.len();
2000+
if address_end <= cast(aux_data_end).to_usize() {
2001+
self.memory_mmap.as_slice_mut()[cast(address).to_usize()..address_end].copy_from_slice(data);
20032002
return Ok(());
20042003
} else {
20052004
false
@@ -2053,8 +2052,10 @@ impl super::Sandbox for Sandbox {
20532052
if !self.dynamic_paging_enabled {
20542053
let memory_map = module.memory_map();
20552054
let is_ok = if address >= memory_map.aux_data_address() {
2056-
if u64::from(address) + u64::from(length) <= u64::from(memory_map.aux_data_range().end) {
2057-
self.memory_mmap.as_slice_mut()[address as usize..address as usize + length as usize].fill(0);
2055+
let aux_data_end = module.memory_map().aux_data_address() + self.aux_data_length;
2056+
let address_end = cast(address).to_usize() + cast(length).to_usize();
2057+
if address_end <= cast(aux_data_end).to_usize() {
2058+
self.memory_mmap.as_slice_mut()[cast(address).to_usize()..address_end].fill(0);
20582059
return Ok(());
20592060
} else {
20602061
false

‎crates/polkavm/src/tests.rs‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2730,7 +2730,89 @@ fn aux_data_accessible_area(config: Config) {
27302730
instance.set_next_program_counter(offsets[0]);
27312731
match_interrupt!(instance.run().unwrap(), InterruptKind::Finished);
27322732

2733+
assert!(instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size - 3, 4, false));
2734+
assert!(instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size * 2 - 4, 4, false));
2735+
assert!(!instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size * 2 - 3, 4, false));
27332736
assert!(instance.read_u32(module.memory_map().aux_data_address() + page_size - 3).is_ok());
2737+
assert!(instance
2738+
.read_u32(module.memory_map().aux_data_address() + page_size * 2 - 4)
2739+
.is_ok());
2740+
assert!(instance
2741+
.read_u32(module.memory_map().aux_data_address() + page_size * 2 - 3)
2742+
.is_err());
2743+
2744+
assert!(instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size - 3, 4, true));
2745+
assert!(instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size * 2 - 4, 4, true));
2746+
assert!(!instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size * 2 - 3, 4, true));
2747+
assert!(!instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size * 2, 4, true));
2748+
assert!(instance
2749+
.write_u32(module.memory_map().aux_data_address() + page_size - 3, 0)
2750+
.is_ok());
2751+
assert!(instance
2752+
.write_u32(module.memory_map().aux_data_address() + page_size * 2 - 4, 0)
2753+
.is_ok());
2754+
assert!(instance
2755+
.write_u32(module.memory_map().aux_data_address() + page_size * 2 - 3, 0)
2756+
.is_err());
2757+
assert!(instance
2758+
.write_u32(module.memory_map().aux_data_address() + page_size * 2, 0)
2759+
.is_err());
2760+
assert!(instance
2761+
.zero_memory(module.memory_map().aux_data_address() + page_size - 3, 4)
2762+
.is_ok());
2763+
assert!(instance
2764+
.zero_memory(module.memory_map().aux_data_address() + page_size * 2 - 4, 4)
2765+
.is_ok());
2766+
assert!(instance
2767+
.zero_memory(module.memory_map().aux_data_address() + page_size * 2 - 3, 4)
2768+
.is_err());
2769+
assert!(instance
2770+
.zero_memory(module.memory_map().aux_data_address() + page_size * 2, 4)
2771+
.is_err());
2772+
2773+
instance.set_host_side_aux_write_protect(true).unwrap();
2774+
2775+
// Still readable as before.
2776+
assert!(instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size - 3, 4, false));
2777+
assert!(instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size * 2 - 4, 4, false));
2778+
assert!(!instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size * 2 - 3, 4, false));
2779+
assert!(instance.read_u32(module.memory_map().aux_data_address() + page_size - 3).is_ok());
2780+
assert!(instance
2781+
.read_u32(module.memory_map().aux_data_address() + page_size * 2 - 4)
2782+
.is_ok());
2783+
assert!(instance
2784+
.read_u32(module.memory_map().aux_data_address() + page_size * 2 - 3)
2785+
.is_err());
2786+
2787+
// Not writable anymore.
2788+
assert!(!instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size - 3, 4, true));
2789+
assert!(!instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size * 2 - 4, 4, true));
2790+
assert!(!instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size * 2 - 3, 4, true));
2791+
assert!(!instance.is_memory_accessible(module.memory_map().aux_data_address() + page_size * 2, 4, true));
2792+
assert!(instance
2793+
.write_u32(module.memory_map().aux_data_address() + page_size - 3, 0)
2794+
.is_err());
2795+
assert!(instance
2796+
.write_u32(module.memory_map().aux_data_address() + page_size * 2 - 4, 0)
2797+
.is_err());
2798+
assert!(instance
2799+
.write_u32(module.memory_map().aux_data_address() + page_size * 2 - 3, 0)
2800+
.is_err());
2801+
assert!(instance
2802+
.write_u32(module.memory_map().aux_data_address() + page_size * 2, 0)
2803+
.is_err());
2804+
assert!(instance
2805+
.zero_memory(module.memory_map().aux_data_address() + page_size - 3, 4)
2806+
.is_err());
2807+
assert!(instance
2808+
.zero_memory(module.memory_map().aux_data_address() + page_size * 2 - 4, 4)
2809+
.is_err());
2810+
assert!(instance
2811+
.zero_memory(module.memory_map().aux_data_address() + page_size * 2 - 3, 4)
2812+
.is_err());
2813+
assert!(instance
2814+
.zero_memory(module.memory_map().aux_data_address() + page_size * 2, 4)
2815+
.is_err());
27342816
}
27352817

27362818
fn access_memory_from_host(config: Config) {

0 commit comments

Comments
 (0)