authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2025-02-05 16:30:46-08:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2025-02-06 14:23:23-08:00
log8282565ce524c14e62e8c30e1feb8afc5c2ab961
tree75d95b7ea0b82840996ddbaa69e9f09a1e08e300
parent6aab1ea2562806124d4add8d95fb758edf914893

std.heap.GeneralPurposeAllocator: fix UAF in resizeLarge

There was an ensureUnusedCapacity() call that invalidated a looked-up hash table entry. Move it earlier.

1 files changed, 19 insertions(+), 16 deletions(-)

lib/std/heap/general_purpose_allocator.zig+19-16
......@@ -532,6 +532,13 @@ pub fn GeneralPurposeAllocator(comptime config: Config) type {
532532 ret_addr: usize,
533533 may_move: bool,
534534 ) ?[*]u8 {
535 if (config.retain_metadata and may_move) {
536 // Before looking up the entry (since this could invalidate
537 // it), we must reserve space for the new entry in case the
538 // allocation is relocated.
539 self.large_allocations.ensureUnusedCapacity(self.backing_allocator, 1) catch return null;
540 }
541
535542 const entry = self.large_allocations.getEntry(@intFromPtr(old_mem.ptr)) orelse {
536543 if (config.safety) {
537544 @panic("Invalid free");
......@@ -584,15 +591,9 @@ pub fn GeneralPurposeAllocator(comptime config: Config) type {
584591 self.total_requested_bytes = new_req_bytes;
585592 }
586593
587 const opt_resized_ptr = if (may_move) b: {
588 // So that if the allocation moves, we can memcpy the
589 // `LargeAlloc` value directly from old to new location.
590 // It's also not clear to me whether removing one item from std
591 // lib hash map guarantees that unused capacity increases by
592 // one.
593 self.large_allocations.ensureUnusedCapacity(self.backing_allocator, 1) catch return null;
594 break :b self.backing_allocator.rawRemap(old_mem, alignment, new_size, ret_addr);
595 } else if (self.backing_allocator.rawResize(old_mem, alignment, new_size, ret_addr))
594 const opt_resized_ptr = if (may_move)
595 self.backing_allocator.rawRemap(old_mem, alignment, new_size, ret_addr)
596 else if (self.backing_allocator.rawResize(old_mem, alignment, new_size, ret_addr))
596597 old_mem.ptr
597598 else
598599 null;
......@@ -619,6 +620,14 @@ pub fn GeneralPurposeAllocator(comptime config: Config) type {
619620
620621 // Update the key of the hash map if the memory was relocated.
621622 if (resized_ptr != old_mem.ptr) {
623 const large_alloc = entry.value_ptr.*;
624 if (config.retain_metadata) {
625 entry.value_ptr.freed = true;
626 entry.value_ptr.captureStackTrace(ret_addr, .free);
627 } else {
628 self.large_allocations.removeByPtr(entry.key_ptr);
629 }
630
622631 const gop = self.large_allocations.getOrPutAssumeCapacity(@intFromPtr(resized_ptr));
623632 if (config.retain_metadata and !config.never_unmap) {
624633 // Backing allocator may be reusing memory that we're retaining metadata for
......@@ -626,13 +635,7 @@ pub fn GeneralPurposeAllocator(comptime config: Config) type {
626635 } else {
627636 assert(!gop.found_existing); // This would mean the kernel double-mapped pages.
628637 }
629 gop.value_ptr.* = entry.value_ptr.*;
630 if (!config.retain_metadata) {
631 self.large_allocations.removeByPtr(entry.key_ptr);
632 } else {
633 entry.value_ptr.freed = true;
634 entry.value_ptr.captureStackTrace(ret_addr, .free);
635 }
638 gop.value_ptr.* = large_alloc;
636639 }
637640
638641 return resized_ptr;