Skip to content

Commit a801db5

Browse files
Fix OffsetModel missing UB for offsets wrapping CBMC's pointer encoding
CBMC encodes pointers as an (object, offset) pair where the offset field has pointer_width - object_bits bits. Pointer arithmetic therefore wraps offsets at 2^(64 - object_bits) rather than 2^64 (#1150). For the OffsetModel this meant that a symbolic byte offset that is a non-zero multiple of 2^(64 - object_bits) (2^48 with Kani's default of 16 object bits) made `wrapping_byte_offset` alias the original pointer, so the subsequent same-allocation safety check passed spuriously and genuine out-of-bounds `offset` calls verified successfully - including "proving" the false fact that the result equals the base pointer. Detect the wrap by additionally requiring that the address of the result is consistent with full-width integer arithmetic on the address of the original pointer; any truncation in the pointer addition falsifies this equation. Two alternative formulations do not work: * Computing the new pointer via integer arithmetic (addr().wrapping_add_signed(off) followed by a cast back, as previously suggested in a FIXME here) is sound, but makes the solver case-split the integer address over every object; harnesses using e.g. `sort()` then run out of memory. These are the "unexpected failures" mentioned in the removed comment (reproduced on: expected/cover/cover-pass, expected/string-repeat, expected/loop-backedge, expected/shadow/unsupported_num_objects). * Comparing POINTER_OFFSET(new) against full-width POINTER_OFFSET(orig) + offset is defeated by CBMC's simplifier, which rewrites POINTER_OFFSET(p + k) to POINTER_OFFSET(p) + k in full-width arithmetic, making the equation trivially true. The `addr()` transmute is opaque to that simplification, keeps all operations in the pointer domain, and adds negligible solver cost: the full expected (459) and kani (593) suites pass unchanged. The underlying encoding wrap is fixed on the CBMC side in diffblue/cbmc#9134 (out-of-range results are directed to the invalid object). The equation here also states Rust's address-arithmetic contract for `offset` and produces the same verdicts with and without that CBMC fix, so it is kept as a semantic guard rather than a version-specific workaround. Partially addresses #1150: this fixes the missed-UB soundness hole in the checked `offset`/`arith_offset` intrinsic model. Not addressed is the `wrapping_offset` family, which Kani codegens to plain CBMC pointer addition: under CBMC versions without the encoding fix its result aliases the base pointer for wrap-multiple offsets (tests/kani/PointerOffset/fixme_wrap_nonzero_offset.rs), and even with the CBMC fix the result address is indeterminate rather than the exactly-wrapped address that Rust defines. Exact modeling would require integer arithmetic plus an integer-to-pointer cast, which makes the solver case-split over all objects (see above). Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
1 parent 1c9134a commit a801db5

3 files changed

Lines changed: 100 additions & 7 deletions

File tree

library/kani_core/src/models.rs

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -124,16 +124,39 @@ macro_rules! generate_models {
124124
let (byte_offset, overflow) = offset.overflowing_mul(t_size);
125125
kani::safety_check(!overflow, "Offset in bytes overflows isize");
126126
let orig_ptr = ptr.to_const_ptr();
127-
// NOTE: For CBMC, using the pointer addition can have unexpected behavior
128-
// when the offset is higher than the object bits since it will wrap around.
129-
// See for more details: https://github.com/model-checking/kani/issues/1150
127+
// The equation below asserts Rust's semantic contract for `offset`:
128+
// the address of the result must equal the address of the original
129+
// pointer plus the byte offset, in full-width (2^64) integer
130+
// arithmetic.
130131
//
131-
// However, when I tried implementing this using usize operation, we got some
132-
// unexpected failures that still require further debugging.
133-
// let new_ptr = orig_ptr.addr().wrapping_add_signed(byte_offset) as *const T;
132+
// Besides documenting that contract, it guards against CBMC's
133+
// pointer encoding wrapping offsets at
134+
// 2^(pointer_width - object_bits) rather than 2^pointer_width: for
135+
// symbolic offsets that are multiples of 2^(64 - object_bits) the
136+
// pointer addition below aliases back into the allocation, so the
137+
// same-allocation check alone would pass spuriously and hide
138+
// genuine UB (https://github.com/model-checking/kani/issues/1150).
139+
// CBMC fixed this in https://github.com/diffblue/cbmc/pull/9134 by
140+
// directing out-of-range results to the invalid object at a
141+
// nondeterministic address (which makes the equation below
142+
// refutable, so this safety check still fails; the two mechanisms
143+
// agree); the guard remains necessary for CBMC versions without
144+
// that fix, and remains correct with it.
145+
//
146+
// Implementation notes:
147+
// * `addr()` (a pointer-to-integer transmute) is deliberately used
148+
// here rather than `kani::mem::pointer_offset`: CBMC's simplifier
149+
// rewrites `POINTER_OFFSET(p + k)` to `POINTER_OFFSET(p) + k` in
150+
// full-width arithmetic, which would make a pointer_offset-based
151+
// equation trivially true and hide the wrap again.
152+
// * This formulation keeps all operations in the pointer domain (no
153+
// integer-to-pointer casts, which make the solver case-split the
154+
// address over every object and blow up, e.g., on harnesses using
155+
// `sort()`).
134156
let new_ptr = orig_ptr.wrapping_byte_offset(byte_offset);
157+
let no_wrap = new_ptr.addr() == orig_ptr.addr().wrapping_add(byte_offset as usize);
135158
kani::safety_check(
136-
kani::mem::same_allocation_internal(orig_ptr, new_ptr),
159+
no_wrap && kani::mem::same_allocation_internal(orig_ptr, new_ptr),
137160
"Offset result and original pointer must point to the same allocation",
138161
);
139162
P::from_const_ptr(new_ptr)
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
Checking harness check_valid_negative_offset...
2+
VERIFICATION:- SUCCESSFUL
3+
4+
Checking harness check_ub_sym_wrap_min...
5+
Failed Checks: Offset result and original pointer must point to the same allocation
6+
VERIFICATION:- FAILED
7+
8+
Checking harness check_ub_sym_wrap_negative...
9+
Failed Checks: Offset result and original pointer must point to the same allocation
10+
VERIFICATION:- FAILED
11+
12+
Checking harness check_ub_sym_wrap_positive...
13+
Failed Checks: Offset result and original pointer must point to the same allocation
14+
VERIFICATION:- FAILED
15+
16+
Complete - 1 successfully verified harnesses, 3 failures, 4 total.
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Copyright Kani Contributors
2+
// SPDX-License-Identifier: Apache-2.0 OR MIT
3+
4+
//! Check that the offset UB check is not defeated by CBMC's pointer-offset
5+
//! encoding, which wraps offsets at 2^(pointer_width - object_bits) rather
6+
//! than 2^pointer_width: a symbolic byte offset that is a non-zero multiple
7+
//! of 2^(64 - object_bits) (e.g. 2^48 with Kani's default of 16 object bits)
8+
//! used to alias the original pointer, making the same-allocation check pass
9+
//! spuriously and hiding genuine UB.
10+
//! See <https://github.com/model-checking/kani/issues/1150>.
11+
//!
12+
//! CBMC fixed the underlying encoding wrap in
13+
//! <https://github.com/diffblue/cbmc/pull/9134> (out-of-range results are
14+
//! directed to the invalid object); these harnesses must produce the same
15+
//! verdicts with and without that fix, via the address-arithmetic guard in
16+
//! Kani's OffsetModel.
17+
18+
#[kani::proof]
19+
fn check_ub_sym_wrap_positive() {
20+
let v = [0u8; 8];
21+
let p: *const u8 = &v[0];
22+
let off: isize = kani::any();
23+
kani::assume(off == 1isize << 48);
24+
let _q = unsafe { p.offset(off) }; // UB: far out of bounds
25+
}
26+
27+
#[kani::proof]
28+
fn check_ub_sym_wrap_negative() {
29+
let v = [0u8; 8];
30+
let p: *const u8 = &v[0];
31+
let off: isize = kani::any();
32+
kani::assume(off == -(1isize << 48));
33+
let _q = unsafe { p.offset(off) }; // UB: far out of bounds
34+
}
35+
36+
#[kani::proof]
37+
fn check_ub_sym_wrap_min() {
38+
let v = [0u8; 8];
39+
let p: *const u8 = &v[0];
40+
let off: isize = kani::any();
41+
kani::assume(off == isize::MIN);
42+
let _q = unsafe { p.byte_offset(off) }; // UB: far out of bounds
43+
}
44+
45+
/// In-bounds negative offsets must still verify, including the extremes.
46+
#[kani::proof]
47+
fn check_valid_negative_offset() {
48+
let v = [0u8; 8];
49+
let p: *const u8 = &v[4];
50+
let off: isize = kani::any();
51+
kani::assume(off >= -4 && off <= 3);
52+
let q = unsafe { p.offset(off) };
53+
assert_eq!(unsafe { q.offset_from(v.as_ptr()) }, 4 + off);
54+
}

0 commit comments

Comments
 (0)