Skip to content

Commit 93f5892

Browse files
committed
Auto merge of #815 - RalfJung:memory-audit, r=RalfJung
don't call Memory::get without checking the pointer first Also avoid Memory::get if we just need to know align/size. I audited all uses of `alloc_id`; the rest should be fine (and we can kill a bunch of them once rust-lang/rust#62257 lands).
2 parents 10af387 + 4135441 commit 93f5892

File tree

2 files changed

+27
-15
lines changed

2 files changed

+27
-15
lines changed

src/operator.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'mir, 'tcx> {
206206
// on read hardware this can easily happen. Thus for comparisons we require
207207
// both pointers to be live.
208208
if self.pointer_inbounds(left).is_ok() && self.pointer_inbounds(right).is_ok() {
209-
// Two in-bounds pointers in different allocations are different.
209+
// Two in-bounds (and hence live) pointers in different allocations are different.
210210
false
211211
} else {
212212
return err!(InvalidPointerMath);
@@ -303,7 +303,9 @@ impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'mir, 'tcx> {
303303
map_to_primval(left.overflowing_offset(Size::from_bytes(right as u64), self)),
304304

305305
BitAnd if !signed => {
306-
let ptr_base_align = self.memory().get(left.alloc_id)?.align.bytes();
306+
let ptr_base_align = self.memory().get_size_and_align(left.alloc_id, AllocCheck::MaybeDead)
307+
.expect("alloc info with MaybeDead cannot fail")
308+
.1.bytes();
307309
let base_mask = {
308310
// FIXME: use `interpret::truncate`, once that takes a `Size` instead of a `Layout`.
309311
let shift = 128 - self.memory().pointer_size().bits();
@@ -337,7 +339,9 @@ impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'mir, 'tcx> {
337339
Rem if !signed => {
338340
// Doing modulo a divisor of the alignment is allowed.
339341
// (Intuition: modulo a divisor leaks less information.)
340-
let ptr_base_align = self.memory().get(left.alloc_id)?.align.bytes();
342+
let ptr_base_align = self.memory().get_size_and_align(left.alloc_id, AllocCheck::MaybeDead)
343+
.expect("alloc info with MaybeDead cannot fail")
344+
.1.bytes();
341345
let right = right as u64;
342346
let ptr_size = self.memory().pointer_size();
343347
if right == 1 {

src/shims/foreign_items.rs

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
227227
Align::from_bytes(align).unwrap(),
228228
MiriMemoryKind::Rust.into()
229229
);
230+
// We just allocated this, the access cannot fail
230231
this.memory_mut()
231-
.get_mut(ptr.alloc_id)?
232-
.write_repeat(tcx, ptr, 0, Size::from_bytes(size))?;
232+
.get_mut(ptr.alloc_id).unwrap()
233+
.write_repeat(tcx, ptr, 0, Size::from_bytes(size)).unwrap();
233234
this.write_scalar(Scalar::Ptr(ptr), dest)?;
234235
}
235236
"__rust_dealloc" => {
@@ -469,15 +470,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
469470
Align::from_bytes(1).unwrap(),
470471
MiriMemoryKind::Env.into(),
471472
);
472-
{
473-
let alloc = this.memory_mut().get_mut(value_copy.alloc_id)?;
474-
alloc.write_bytes(tcx, value_copy, &value)?;
475-
let trailing_zero_ptr = value_copy.offset(
476-
Size::from_bytes(value.len() as u64),
477-
tcx,
478-
)?;
479-
alloc.write_bytes(tcx, trailing_zero_ptr, &[0])?;
480-
}
473+
// We just allocated these, so the write cannot fail.
474+
let alloc = this.memory_mut().get_mut(value_copy.alloc_id).unwrap();
475+
alloc.write_bytes(tcx, value_copy, &value).unwrap();
476+
let trailing_zero_ptr = value_copy.offset(
477+
Size::from_bytes(value.len() as u64),
478+
tcx,
479+
).unwrap();
480+
alloc.write_bytes(tcx, trailing_zero_ptr, &[0]).unwrap();
481+
481482
if let Some(var) = this.machine.env_vars.insert(
482483
name.to_owned(),
483484
value_copy,
@@ -814,7 +815,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
814815
},
815816
"GetSystemInfo" => {
816817
let system_info = this.deref_operand(args[0])?;
817-
let system_info_ptr = system_info.ptr.to_ptr()?;
818+
let (system_info_ptr, align) = system_info.to_scalar_ptr_align();
819+
let system_info_ptr = this.memory()
820+
.check_ptr_access(
821+
system_info_ptr,
822+
system_info.layout.size,
823+
align,
824+
)?
825+
.expect("cannot be a ZST");
818826
// Initialize with `0`.
819827
this.memory_mut().get_mut(system_info_ptr.alloc_id)?
820828
.write_repeat(tcx, system_info_ptr, 0, system_info.layout.size)?;

0 commit comments

Comments
 (0)