authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2023-06-13 10:52:41-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2023-06-13 10:52:41-07:00
log8cf72cdfb13409aad9a5e2abc125a516e13c3aa6
tree2b81891a33f5ef9b975314d42f690c4a7b211492
parent02df9aa9762bac2217b44121a5bae3afccb5ba71
parent89bd29a9058eabf23c735761c49809bb63a68842
signaturebadge-question-mark Signed by PGP key 4AEE18F83AFDEB23

Merge pull request #15985 from erikarvstedt/fix-arena-alloc

arena_allocator/reset: fix use after free, fix buffer overrun

1 files changed, 24 insertions(+), 10 deletions(-)

lib/std/heap/arena_allocator.zig+24-10
......@@ -108,21 +108,18 @@ pub const ArenaAllocator = struct {
108108 // Thus, only the first hand full of calls to reset() will actually need to iterate the linked
109109 // list, all future calls are just taking the first node, and only resetting the `end_index`
110110 // value.
111 const current_capacity = if (mode != .free_all)
112 @sizeOf(BufNode) + self.queryCapacity() // we need at least space for exactly one node + the current capacity
113 else
114 0;
115 if (mode == .free_all or current_capacity == 0) {
111 const requested_capacity = switch (mode) {
112 .retain_capacity => self.queryCapacity(),
113 .retain_with_limit => |limit| std.math.min(limit, self.queryCapacity()),
114 .free_all => 0,
115 };
116 if (requested_capacity == 0) {
116117 // just reset when we don't have anything to reallocate
117118 self.deinit();
118119 self.state = State{};
119120 return true;
120121 }
121 const total_size = switch (mode) {
122 .retain_capacity => current_capacity,
123 .retain_with_limit => |limit| std.math.min(limit, current_capacity),
124 .free_all => unreachable,
125 };
122 const total_size = requested_capacity + @sizeOf(BufNode);
126123 const align_bits = std.math.log2_int(usize, @alignOf(BufNode));
127124 // Free all nodes except for the last one
128125 var it = self.state.buffer_list.first;
......@@ -139,6 +136,7 @@ pub const ArenaAllocator = struct {
139136 // reset the state before we try resizing the buffers, so we definitely have reset the arena to 0.
140137 self.state.end_index = 0;
141138 if (maybe_first_node) |first_node| {
139 self.state.buffer_list.first = first_node;
142140 // perfect, no need to invoke the child_allocator
143141 if (first_node.data == total_size)
144142 return true;
......@@ -270,3 +268,19 @@ test "ArenaAllocator (reset with preheating)" {
270268 }
271269 }
272270}
271
272test "ArenaAllocator (reset while retaining a buffer)" {
273 var arena_allocator = ArenaAllocator.init(std.testing.allocator);
274 defer arena_allocator.deinit();
275 const a = arena_allocator.allocator();
276
277 // Create two internal buffers
278 _ = try a.alloc(u8, 1);
279 _ = try a.alloc(u8, 1000);
280
281 // Check that we have at least two buffers
282 try std.testing.expect(arena_allocator.state.buffer_list.first.?.next != null);
283
284 // This retains the first allocated buffer
285 try std.testing.expect(arena_allocator.reset(.{ .retain_with_limit = 1 }));
286}