Skip to content

Commit

Permalink
Make Kani reject mutable pointer casts if padding is incompatible and…
Browse files Browse the repository at this point in the history
… memory initialization is checked (model-checking#3332)

This PR introduces layout checks for types to instrument mutable pointer
casts. If two types have incompatible padding (e.g. a padding byte in
one is a data byte in the other or vice-versa), an "unsupported check"
assertion is inserted. This overapproximates for soundness, since the
casts do not cause UB themselves, but an alternative solution involves
tracking every MIR place, which is costly.

Resolves model-checking#3324

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
  • Loading branch information
artemagvanian authored Jul 9, 2024
1 parent 34820bd commit 923346c
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ use stable_mir::mir::{
NonDivergingIntrinsic, Operand, Place, PointerCoercion, ProjectionElem, Rvalue, Statement,
StatementKind, Terminator, TerminatorKind,
};
use stable_mir::ty::{ConstantKind, MirConst, RigidTy, Span, TyKind, UintTy};
use stable_mir::ty::{ConstantKind, MirConst, RigidTy, Span, Ty, TyKind, UintTy};
use strum_macros::AsRefStr;

use super::{PointeeInfo, PointeeLayout};

/// Memory initialization operations: set or get memory initialization state for a given pointer.
#[derive(AsRefStr, Clone, Debug)]
pub enum MemoryInitOp {
Expand Down Expand Up @@ -540,13 +542,34 @@ impl<'a> MirVisitor for CheckUninitVisitor<'a> {
}

fn visit_rvalue(&mut self, rvalue: &Rvalue, location: Location) {
if let Rvalue::Cast(CastKind::PointerCoercion(PointerCoercion::Unsize), _, ty) = rvalue {
if let TyKind::RigidTy(RigidTy::RawPtr(pointee_ty, _)) = ty.kind() {
if pointee_ty.kind().is_trait() {
self.push_target(MemoryInitOp::Unsupported {
reason: "Kani does not support reasoning about memory initialization of unsized pointers.".to_string(),
});
if let Rvalue::Cast(cast_kind, operand, ty) = rvalue {
match cast_kind {
CastKind::PointerCoercion(PointerCoercion::Unsize) => {
if let TyKind::RigidTy(RigidTy::RawPtr(pointee_ty, _)) = ty.kind() {
if pointee_ty.kind().is_trait() {
self.push_target(MemoryInitOp::Unsupported {
reason: "Kani does not support reasoning about memory initialization of unsized pointers.".to_string(),
});
}
}
}
CastKind::PtrToPtr => {
let operand_ty = operand.ty(&self.locals).unwrap();
if let (
RigidTy::RawPtr(from_ty, Mutability::Mut),
RigidTy::RawPtr(to_ty, Mutability::Mut),
) = (operand_ty.kind().rigid().unwrap(), ty.kind().rigid().unwrap())
{
if !tys_layout_compatible(from_ty, to_ty) {
// If casting from a mutable pointer to a mutable pointer with
// different layouts, delayed UB could occur.
self.push_target(MemoryInitOp::Unsupported {
reason: "Kani does not support reasoning about memory initialization in presence of mutable raw pointer casts that could cause delayed UB.".to_string(),
});
}
}
}
_ => {}
}
};
self.super_rvalue(rvalue, location);
Expand Down Expand Up @@ -711,3 +734,36 @@ fn try_resolve_instance(locals: &[LocalDecl], func: &Operand) -> Result<Instance
)),
}
}

/// Returns true if `to_ty` has a smaller or equal size and the same padding bytes as `from_ty` up until
/// its size.
fn tys_layout_compatible(from_ty: &Ty, to_ty: &Ty) -> bool {
// Retrieve layouts to assess compatibility.
let from_ty_info = PointeeInfo::from_ty(*from_ty);
let to_ty_info = PointeeInfo::from_ty(*to_ty);
if let (Ok(from_ty_info), Ok(to_ty_info)) = (from_ty_info, to_ty_info) {
let from_ty_layout = match from_ty_info.layout() {
PointeeLayout::Sized { layout } => layout,
PointeeLayout::Slice { element_layout } => element_layout,
PointeeLayout::TraitObject => return false,
};
let to_ty_layout = match to_ty_info.layout() {
PointeeLayout::Sized { layout } => layout,
PointeeLayout::Slice { element_layout } => element_layout,
PointeeLayout::TraitObject => return false,
};
// Ensure `to_ty_layout` does not have a larger size.
if to_ty_layout.len() <= from_ty_layout.len() {
// Check data and padding bytes pair-wise.
if from_ty_layout.iter().zip(to_ty_layout.iter()).all(
|(from_ty_layout_byte, to_ty_layout_byte)| {
// Make sure all data and padding bytes match.
from_ty_layout_byte == to_ty_layout_byte
},
) {
return true;
}
}
};
false
}
2 changes: 1 addition & 1 deletion tests/expected/uninit/access-padding-via-cast/expected
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Failed Checks: Undefined Behavior: Reading from an uninitialized pointer of type `*mut [u8; 4]`
Failed Checks: Kani does not support reasoning about memory initialization in presence of mutable raw pointer casts that could cause delayed UB.

VERIFICATION:- FAILED

Expand Down
14 changes: 14 additions & 0 deletions tests/expected/uninit/delayed-ub/delayed-ub.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright Kani Contributors
// SPDX-License-Identifier: Apache-2.0 OR MIT
// kani-flags: -Z ghost-state -Z uninit-checks

/// Checks that Kani rejects mutable pointer casts between types of different padding.
#[kani::proof]
fn invalid_value() {
unsafe {
let mut value: u128 = 0;
let ptr = &mut value as *mut _ as *mut (u8, u32, u64);
*ptr = (4, 4, 4); // This assignment itself does not cause UB...
let c: u128 = value; // ...but this reads a padding value!
}
}
5 changes: 5 additions & 0 deletions tests/expected/uninit/delayed-ub/expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Failed Checks: Kani does not support reasoning about memory initialization in presence of mutable raw pointer casts that could cause delayed UB.

VERIFICATION:- FAILED

Complete - 0 successfully verified harnesses, 1 failures, 1 total.

0 comments on commit 923346c

Please sign in to comment.