From b0b48511dae63b2e86d002930e54cef7c0d6009e Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sat, 23 Aug 2014 19:22:36 +0200 Subject: [PATCH 1/2] Test case to illustate/reproduce bug. --- src/test/run-pass/realloc-16687.rs | 167 +++++++++++++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 src/test/run-pass/realloc-16687.rs diff --git a/src/test/run-pass/realloc-16687.rs b/src/test/run-pass/realloc-16687.rs new file mode 100644 index 00000000000..2e8c23fe5ba --- /dev/null +++ b/src/test/run-pass/realloc-16687.rs @@ -0,0 +1,167 @@ +// Copyright 2013-2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// alloc::heap::reallocate test. +// +// Ideally this would be revised to use no_std, but for now it serves +// well enough to reproduce (and illustrate) the bug from #16687. + +extern crate alloc; + +use alloc::heap; +use std::ptr; + +fn main() { + unsafe { + assert!(test_triangle()); + } +} + +unsafe fn test_triangle() -> bool { + static COUNT : uint = 16; + let mut ascend = Vec::from_elem(COUNT, ptr::mut_null()); + let ascend = ascend.as_mut_slice(); + static ALIGN : uint = 1; + + // Checks that `ascend` forms triangle of acending size formed + // from pairs of rows (where each pair of rows is equally sized), + // and the elements of the triangle match their row-pair index. + unsafe fn sanity_check(ascend: &[*mut u8]) { + for i in range(0u, COUNT / 2) { + let (p0, p1, size) = (ascend[2*i], ascend[2*i+1], idx_to_size(i)); + for j in range(0u, size) { + assert_eq!(*p0.offset(j as int), i as u8); + assert_eq!(*p1.offset(j as int), i as u8); + } + } + } + + static PRINT : bool = false; + + unsafe fn allocate(size: uint, align: uint) -> *mut u8 { + if PRINT { println!("allocate(size={:u} align={:u})", size, align); } + + let ret = heap::allocate(size, align); + + if PRINT { println!("allocate(size={:u} align={:u}) ret: 0x{:010x}", + size, align, ret as uint); + } + + ret + } + unsafe fn reallocate(ptr: *mut u8, size: uint, align: uint, + old_size: uint) -> *mut u8 { + if PRINT { + println!("reallocate(ptr=0x{:010x} size={:u} align={:u} old_size={:u})", + ptr as uint, size, align, old_size); + } + + let ret = heap::reallocate(ptr, size, align, old_size); + + if PRINT { + println!("reallocate(ptr=0x{:010x} size={:u} align={:u} old_size={:u}) \ + ret: 0x{:010x}", + ptr as uint, size, align, old_size, ret as uint); + } + ret + } + + fn idx_to_size(i: uint) -> uint { (i+1) * 10 } + + // Allocate pairs of rows that form a triangle shape. (Hope is + // that at least two rows will be allocated near each other, so + // that we trigger the bug (a buffer overrun) in an observable + // way.) + for i in range(0u, COUNT / 2) { + let size = idx_to_size(i); + ascend[2*i] = allocate(size, ALIGN); + ascend[2*i+1] = allocate(size, ALIGN); + } + + // Initialize each pair of rows to distinct value. + for i in range(0u, COUNT / 2) { + let (p0, p1, size) = (ascend[2*i], ascend[2*i+1], idx_to_size(i)); + for j in range(0, size) { + *p0.offset(j as int) = i as u8; + *p1.offset(j as int) = i as u8; + } + } + + sanity_check(ascend.as_slice()); + test_1(ascend); + test_2(ascend); + test_3(ascend); + test_4(ascend); + + return true; + + // Test 1: turn the triangle into a square (in terms of + // allocation; initialized portion remains a triangle) by + // realloc'ing each row from top to bottom, and checking all the + // rows as we go. + unsafe fn test_1(ascend: &mut [*mut u8]) { + let new_size = idx_to_size(COUNT-1); + for i in range(0u, COUNT / 2) { + let (p0, p1, old_size) = (ascend[2*i], ascend[2*i+1], idx_to_size(i)); + assert!(old_size < new_size); + + ascend[2*i] = reallocate(p0, new_size, ALIGN, old_size); + sanity_check(ascend.as_slice()); + + ascend[2*i+1] = reallocate(p1, new_size, ALIGN, old_size); + sanity_check(ascend.as_slice()); + } + } + + // Test 2: turn the square back into a triangle, top to bottom. + unsafe fn test_2(ascend: &mut [*mut u8]) { + let old_size = idx_to_size(COUNT-1); + for i in range(0u, COUNT / 2) { + let (p0, p1, new_size) = (ascend[2*i], ascend[2*i+1], idx_to_size(i)); + assert!(new_size < old_size); + + ascend[2*i] = reallocate(p0, new_size, ALIGN, old_size); + sanity_check(ascend.as_slice()); + + ascend[2*i+1] = reallocate(p1, new_size, ALIGN, old_size); + sanity_check(ascend.as_slice()); + } + } + + // Test 3: turn triangle into a square, bottom to top. + unsafe fn test_3(ascend: &mut [*mut u8]) { + let new_size = idx_to_size(COUNT-1); + for i in range(0u, COUNT / 2).rev() { + let (p0, p1, old_size) = (ascend[2*i], ascend[2*i+1], idx_to_size(i)); + assert!(old_size < new_size); + + ascend[2*i+1] = reallocate(p1, new_size, ALIGN, old_size); + sanity_check(ascend.as_slice()); + + ascend[2*i] = reallocate(p0, new_size, ALIGN, old_size); + sanity_check(ascend.as_slice()); + } + } + + // Test 4: turn the square back into a triangle, bottom to top. + unsafe fn test_4(ascend: &mut [*mut u8]) { + let old_size = idx_to_size(COUNT-1); + for i in range(0u, COUNT / 2).rev() { + let (p0, p1, new_size) = (ascend[2*i], ascend[2*i+1], idx_to_size(i)); + assert!(new_size < old_size); + + ascend[2*i+1] = reallocate(p1, new_size, ALIGN, old_size); + sanity_check(ascend.as_slice()); + + ascend[2*i] = reallocate(p0, new_size, ALIGN, old_size); + sanity_check(ascend.as_slice()); + } + } +} From b1f7d3aaa021d626b4083ddaa706b26f3521d343 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sat, 23 Aug 2014 17:29:48 +0200 Subject: [PATCH 2/2] Copy only up to `min(new_size, old_size)` when doing reallocate. Fix #16687 --- src/liballoc/heap.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/liballoc/heap.rs b/src/liballoc/heap.rs index e2faa3240ed..ab686cb01d6 100644 --- a/src/liballoc/heap.rs +++ b/src/liballoc/heap.rs @@ -208,6 +208,7 @@ mod imp { #[cfg(not(jemalloc), unix)] mod imp { + use core::cmp; use core::mem; use core::ptr; use libc; @@ -248,7 +249,7 @@ mod imp { pub unsafe fn reallocate(ptr: *mut u8, size: uint, align: uint, old_size: uint) -> *mut u8 { let new_ptr = allocate(size, align); - ptr::copy_memory(new_ptr, ptr as *const u8, old_size); + ptr::copy_memory(new_ptr, ptr as *const u8, cmp::min(size, old_size)); deallocate(ptr, old_size, align); return new_ptr; }