Skip to content

Commit 48ebad5

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of #83065 - CDirkx:win-alloc, r=dtolnay
Rework `std::sys::windows::alloc` I came across #76676 (comment), which points out that there was unsound code in the Windows alloc code, creating a &mut to possibly uninitialized memory. I reworked the code so that that particular issue does not occur anymore, and started adding more documentation and safety comments. Full list of changes: - moved and documented the relevant Windows Heap API functions - refactor `allocate_with_flags` to `allocate` (and remove the other helper functions), which now takes just a `bool` if the memory should be zeroed - add checks for if `GetProcessHeap` returned null - add a test that checks if the size and alignment of a `Header` are indeed <= `MIN_ALIGN` - add `#![deny(unsafe_op_in_unsafe_fn)]` and the necessary unsafe blocks with safety comments I feel like I may have overdone the documenting, the unsoundness fix is the most important part; I could spit this PR up in separate parts.
2 parents 36bcf40 + db1d003 commit 48ebad5

File tree

3 files changed

+220
-33
lines changed

3 files changed

+220
-33
lines changed

‎library/std/src/sys/windows/alloc.rs

+211-26
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,246 @@
1+
#![deny(unsafe_op_in_unsafe_fn)]
2+
13
use crate::alloc::{GlobalAlloc, Layout, System};
4+
use crate::ffi::c_void;
5+
use crate::ptr;
6+
use crate::sync::atomic::{AtomicPtr, Ordering};
27
use crate::sys::c;
38
use crate::sys_common::alloc::{realloc_fallback, MIN_ALIGN};
49

5-
#[repr(C)]
6-
struct Header(*mut u8);
10+
#[cfg(test)]
11+
mod tests;
12+
13+
// Heap memory management on Windows is done by using the system Heap API (heapapi.h)
14+
// See https://docs.microsoft.com/windows/win32/api/heapapi/
15+
16+
// Flag to indicate that the memory returned by `HeapAlloc` should be zeroed.
17+
const HEAP_ZERO_MEMORY: c::DWORD = 0x00000008;
718

8-
unsafe fn get_header<'a>(ptr: *mut u8) -> &'a mut Header {
9-
&mut *(ptr as *mut Header).offset(-1)
19+
extern "system" {
20+
// Get a handle to the default heap of the current process, or null if the operation fails.
21+
//
22+
// SAFETY: Successful calls to this function within the same process are assumed to
23+
// always return the same handle, which remains valid for the entire lifetime of the process.
24+
//
25+
// See https://docs.microsoft.com/windows/win32/api/heapapi/nf-heapapi-getprocessheap
26+
fn GetProcessHeap() -> c::HANDLE;
27+
28+
// Allocate a block of `dwBytes` bytes of memory from a given heap `hHeap`.
29+
// The allocated memory may be uninitialized, or zeroed if `dwFlags` is
30+
// set to `HEAP_ZERO_MEMORY`.
31+
//
32+
// Returns a pointer to the newly-allocated memory or null if the operation fails.
33+
// The returned pointer will be aligned to at least `MIN_ALIGN`.
34+
//
35+
// SAFETY:
36+
// - `hHeap` must be a non-null handle returned by `GetProcessHeap`.
37+
// - `dwFlags` must be set to either zero or `HEAP_ZERO_MEMORY`.
38+
//
39+
// Note that `dwBytes` is allowed to be zero, contrary to some other allocators.
40+
//
41+
// See https://docs.microsoft.com/windows/win32/api/heapapi/nf-heapapi-heapalloc
42+
fn HeapAlloc(hHeap: c::HANDLE, dwFlags: c::DWORD, dwBytes: c::SIZE_T) -> c::LPVOID;
43+
44+
// Reallocate a block of memory behind a given pointer `lpMem` from a given heap `hHeap`,
45+
// to a block of at least `dwBytes` bytes, either shrinking the block in place,
46+
// or allocating at a new location, copying memory, and freeing the original location.
47+
//
48+
// Returns a pointer to the reallocated memory or null if the operation fails.
49+
// The returned pointer will be aligned to at least `MIN_ALIGN`.
50+
// If the operation fails the given block will never have been freed.
51+
//
52+
// SAFETY:
53+
// - `hHeap` must be a non-null handle returned by `GetProcessHeap`.
54+
// - `dwFlags` must be set to zero.
55+
// - `lpMem` must be a non-null pointer to an allocated block returned by `HeapAlloc` or
56+
// `HeapReAlloc`, that has not already been freed.
57+
// If the block was successfully reallocated at a new location, pointers pointing to
58+
// the freed memory, such as `lpMem`, must not be dereferenced ever again.
59+
//
60+
// Note that `dwBytes` is allowed to be zero, contrary to some other allocators.
61+
//
62+
// See https://docs.microsoft.com/windows/win32/api/heapapi/nf-heapapi-heaprealloc
63+
fn HeapReAlloc(
64+
hHeap: c::HANDLE,
65+
dwFlags: c::DWORD,
66+
lpMem: c::LPVOID,
67+
dwBytes: c::SIZE_T,
68+
) -> c::LPVOID;
69+
70+
// Free a block of memory behind a given pointer `lpMem` from a given heap `hHeap`.
71+
// Returns a nonzero value if the operation is successful, and zero if the operation fails.
72+
//
73+
// SAFETY:
74+
// - `hHeap` must be a non-null handle returned by `GetProcessHeap`.
75+
// - `dwFlags` must be set to zero.
76+
// - `lpMem` must be a pointer to an allocated block returned by `HeapAlloc` or `HeapReAlloc`,
77+
// that has not already been freed.
78+
// If the block was successfully freed, pointers pointing to the freed memory, such as `lpMem`,
79+
// must not be dereferenced ever again.
80+
//
81+
// Note that `lpMem` is allowed to be null, which will not cause the operation to fail.
82+
//
83+
// See https://docs.microsoft.com/windows/win32/api/heapapi/nf-heapapi-heapfree
84+
fn HeapFree(hHeap: c::HANDLE, dwFlags: c::DWORD, lpMem: c::LPVOID) -> c::BOOL;
1085
}
1186

12-
unsafe fn align_ptr(ptr: *mut u8, align: usize) -> *mut u8 {
13-
let aligned = ptr.add(align - (ptr as usize & (align - 1)));
14-
*get_header(aligned) = Header(ptr);
15-
aligned
87+
// Cached handle to the default heap of the current process.
88+
// Either a non-null handle returned by `GetProcessHeap`, or null when not yet initialized or `GetProcessHeap` failed.
89+
static HEAP: AtomicPtr<c_void> = AtomicPtr::new(ptr::null_mut());
90+
91+
// Get a handle to the default heap of the current process, or null if the operation fails.
92+
// If this operation is successful, `HEAP` will be successfully initialized and contain
93+
// a non-null handle returned by `GetProcessHeap`.
94+
#[inline]
95+
fn init_or_get_process_heap() -> c::HANDLE {
96+
let heap = HEAP.load(Ordering::Relaxed);
97+
if heap.is_null() {
98+
// `HEAP` has not yet been successfully initialized
99+
let heap = unsafe { GetProcessHeap() };
100+
if !heap.is_null() {
101+
// SAFETY: No locking is needed because within the same process,
102+
// successful calls to `GetProcessHeap` will always return the same value, even on different threads.
103+
HEAP.store(heap, Ordering::Release);
104+
105+
// SAFETY: `HEAP` contains a non-null handle returned by `GetProcessHeap`
106+
heap
107+
} else {
108+
// Could not get the current process heap.
109+
ptr::null_mut()
110+
}
111+
} else {
112+
// SAFETY: `HEAP` contains a non-null handle returned by `GetProcessHeap`
113+
heap
114+
}
16115
}
17116

117+
// Get a non-null handle to the default heap of the current process.
118+
// SAFETY: `HEAP` must have been successfully initialized.
18119
#[inline]
19-
unsafe fn allocate_with_flags(layout: Layout, flags: c::DWORD) -> *mut u8 {
20-
if layout.align() <= MIN_ALIGN {
21-
return c::HeapAlloc(c::GetProcessHeap(), flags, layout.size()) as *mut u8;
120+
unsafe fn get_process_heap() -> c::HANDLE {
121+
HEAP.load(Ordering::Acquire)
122+
}
123+
124+
// Header containing a pointer to the start of an allocated block.
125+
// SAFETY: Size and alignment must be <= `MIN_ALIGN`.
126+
#[repr(C)]
127+
struct Header(*mut u8);
128+
129+
// Allocate a block of optionally zeroed memory for a given `layout`.
130+
// SAFETY: Returns a pointer satisfying the guarantees of `System` about allocated pointers,
131+
// or null if the operation fails. If this returns non-null `HEAP` will have been successfully
132+
// initialized.
133+
#[inline]
134+
unsafe fn allocate(layout: Layout, zeroed: bool) -> *mut u8 {
135+
let heap = init_or_get_process_heap();
136+
if heap.is_null() {
137+
// Allocation has failed, could not get the current process heap.
138+
return ptr::null_mut();
22139
}
23140

24-
let size = layout.size() + layout.align();
25-
let ptr = c::HeapAlloc(c::GetProcessHeap(), flags, size);
26-
if ptr.is_null() { ptr as *mut u8 } else { align_ptr(ptr as *mut u8, layout.align()) }
141+
// Allocated memory will be either zeroed or uninitialized.
142+
let flags = if zeroed { HEAP_ZERO_MEMORY } else { 0 };
143+
144+
if layout.align() <= MIN_ALIGN {
145+
// SAFETY: `heap` is a non-null handle returned by `GetProcessHeap`.
146+
// The returned pointer points to the start of an allocated block.
147+
unsafe { HeapAlloc(heap, flags, layout.size()) as *mut u8 }
148+
} else {
149+
// Allocate extra padding in order to be able to satisfy the alignment.
150+
let total = layout.align() + layout.size();
151+
152+
// SAFETY: `heap` is a non-null handle returned by `GetProcessHeap`.
153+
let ptr = unsafe { HeapAlloc(heap, flags, total) as *mut u8 };
154+
if ptr.is_null() {
155+
// Allocation has failed.
156+
return ptr::null_mut();
157+
}
158+
159+
// Create a correctly aligned pointer offset from the start of the allocated block,
160+
// and write a header before it.
161+
162+
let offset = layout.align() - (ptr as usize & (layout.align() - 1));
163+
// SAFETY: `MIN_ALIGN` <= `offset` <= `layout.align()` and the size of the allocated
164+
// block is `layout.align() + layout.size()`. `aligned` will thus be a correctly aligned
165+
// pointer inside the allocated block with at least `layout.size()` bytes after it and at
166+
// least `MIN_ALIGN` bytes of padding before it.
167+
let aligned = unsafe { ptr.add(offset) };
168+
// SAFETY: Because the size and alignment of a header is <= `MIN_ALIGN` and `aligned`
169+
// is aligned to at least `MIN_ALIGN` and has at least `MIN_ALIGN` bytes of padding before
170+
// it, it is safe to write a header directly before it.
171+
unsafe { ptr::write((aligned as *mut Header).offset(-1), Header(ptr)) };
172+
173+
// SAFETY: The returned pointer does not point to the to the start of an allocated block,
174+
// but there is a header readable directly before it containing the location of the start
175+
// of the block.
176+
aligned
177+
}
27178
}
28179

180+
// All pointers returned by this allocator have, in addition to the guarantees of `GlobalAlloc`, the
181+
// following properties:
182+
//
183+
// If the pointer was allocated or reallocated with a `layout` specifying an alignment <= `MIN_ALIGN`
184+
// the pointer will be aligned to at least `MIN_ALIGN` and point to the start of the allocated block.
185+
//
186+
// If the pointer was allocated or reallocated with a `layout` specifying an alignment > `MIN_ALIGN`
187+
// the pointer will be aligned to the specified alignment and not point to the start of the allocated block.
188+
// Instead there will be a header readable directly before the returned pointer, containing the actual
189+
// location of the start of the block.
29190
#[stable(feature = "alloc_system_type", since = "1.28.0")]
30191
unsafe impl GlobalAlloc for System {
31192
#[inline]
32193
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
33-
allocate_with_flags(layout, 0)
194+
// SAFETY: Pointers returned by `allocate` satisfy the guarantees of `System`
195+
let zeroed = false;
196+
unsafe { allocate(layout, zeroed) }
34197
}
35198

36199
#[inline]
37200
unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
38-
allocate_with_flags(layout, c::HEAP_ZERO_MEMORY)
201+
// SAFETY: Pointers returned by `allocate` satisfy the guarantees of `System`
202+
let zeroed = true;
203+
unsafe { allocate(layout, zeroed) }
39204
}
40205

41206
#[inline]
42207
unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
43-
if layout.align() <= MIN_ALIGN {
44-
let err = c::HeapFree(c::GetProcessHeap(), 0, ptr as c::LPVOID);
45-
debug_assert!(err != 0, "Failed to free heap memory: {}", c::GetLastError());
46-
} else {
47-
let header = get_header(ptr);
48-
let err = c::HeapFree(c::GetProcessHeap(), 0, header.0 as c::LPVOID);
49-
debug_assert!(err != 0, "Failed to free heap memory: {}", c::GetLastError());
50-
}
208+
let block = {
209+
if layout.align() <= MIN_ALIGN {
210+
ptr
211+
} else {
212+
// The location of the start of the block is stored in the padding before `ptr`.
213+
214+
// SAFETY: Because of the contract of `System`, `ptr` is guaranteed to be non-null
215+
// and have a header readable directly before it.
216+
unsafe { ptr::read((ptr as *mut Header).offset(-1)).0 }
217+
}
218+
};
219+
220+
// SAFETY: because `ptr` has been successfully allocated with this allocator,
221+
// `HEAP` must have been successfully initialized.
222+
let heap = unsafe { get_process_heap() };
223+
224+
// SAFETY: `heap` is a non-null handle returned by `GetProcessHeap`,
225+
// `block` is a pointer to the start of an allocated block.
226+
unsafe { HeapFree(heap, 0, block as c::LPVOID) };
51227
}
52228

53229
#[inline]
54230
unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
55231
if layout.align() <= MIN_ALIGN {
56-
c::HeapReAlloc(c::GetProcessHeap(), 0, ptr as c::LPVOID, new_size) as *mut u8
232+
// SAFETY: because `ptr` has been successfully allocated with this allocator,
233+
// `HEAP` must have been successfully initialized.
234+
let heap = unsafe { get_process_heap() };
235+
236+
// SAFETY: `heap` is a non-null handle returned by `GetProcessHeap`,
237+
// `ptr` is a pointer to the start of an allocated block.
238+
// The returned pointer points to the start of an allocated block.
239+
unsafe { HeapReAlloc(heap, 0, ptr as c::LPVOID, new_size) as *mut u8 }
57240
} else {
58-
realloc_fallback(self, ptr, layout, new_size)
241+
// SAFETY: `realloc_fallback` is implemented using `dealloc` and `alloc`, which will
242+
// correctly handle `ptr` and return a pointer satisfying the guarantees of `System`
243+
unsafe { realloc_fallback(self, ptr, layout, new_size) }
59244
}
60245
}
61246
}

‎library/std/src/sys/windows/alloc/tests.rs

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
use super::{Header, MIN_ALIGN};
2+
use crate::mem;
3+
4+
#[test]
5+
fn alloc_header() {
6+
// Header must fit in the padding before an aligned pointer
7+
assert!(mem::size_of::<Header>() <= MIN_ALIGN);
8+
assert!(mem::align_of::<Header>() <= MIN_ALIGN);
9+
}

‎library/std/src/sys/windows/c.rs

-7
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,6 @@ pub const FD_SETSIZE: usize = 64;
285285

286286
pub const STACK_SIZE_PARAM_IS_A_RESERVATION: DWORD = 0x00010000;
287287

288-
pub const HEAP_ZERO_MEMORY: DWORD = 0x00000008;
289-
290288
pub const STATUS_SUCCESS: NTSTATUS = 0x00000000;
291289

292290
#[repr(C)]
@@ -1017,11 +1015,6 @@ extern "system" {
10171015
timeout: *const timeval,
10181016
) -> c_int;
10191017

1020-
pub fn GetProcessHeap() -> HANDLE;
1021-
pub fn HeapAlloc(hHeap: HANDLE, dwFlags: DWORD, dwBytes: SIZE_T) -> LPVOID;
1022-
pub fn HeapReAlloc(hHeap: HANDLE, dwFlags: DWORD, lpMem: LPVOID, dwBytes: SIZE_T) -> LPVOID;
1023-
pub fn HeapFree(hHeap: HANDLE, dwFlags: DWORD, lpMem: LPVOID) -> BOOL;
1024-
10251018
// >= Vista / Server 2008
10261019
// https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createsymboliclinkw
10271020
pub fn CreateSymbolicLinkW(

0 commit comments

Comments
 (0)