Skip to content

Commit 3106a23

Browse files
authored
[libc][stdlib] Fix UB in freelist (#95330)
Some of the freelist code uses type punning which is UB in C++, namely because we read from a union member that is not the active union member.
1 parent 8fa7cf0 commit 3106a23

File tree

2 files changed

+20
-36
lines changed

2 files changed

+20
-36
lines changed

libc/src/stdlib/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,8 +398,9 @@ else()
398398
freelist.h
399399
DEPENDS
400400
libc.src.__support.fixedvector
401-
libc.src.__support.CPP.cstddef
402401
libc.src.__support.CPP.array
402+
libc.src.__support.CPP.cstddef
403+
libc.src.__support.CPP.new
403404
libc.src.__support.CPP.span
404405
)
405406
add_header_library(

libc/src/stdlib/freelist.h

Lines changed: 18 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include "src/__support/CPP/array.h"
1313
#include "src/__support/CPP/cstddef.h"
14+
#include "src/__support/CPP/new.h"
1415
#include "src/__support/CPP/span.h"
1516
#include "src/__support/fixedvector.h"
1617

@@ -92,19 +93,12 @@ bool FreeList<NUM_BUCKETS>::add_chunk(span<cpp::byte> chunk) {
9293
if (chunk.size() < sizeof(FreeListNode))
9394
return false;
9495

95-
union {
96-
FreeListNode *node;
97-
cpp::byte *bytes;
98-
} aliased;
99-
100-
aliased.bytes = chunk.data();
101-
96+
// Add it to the correct list.
10297
size_t chunk_ptr = find_chunk_ptr_for_size(chunk.size(), false);
10398

104-
// Add it to the correct list.
105-
aliased.node->size = chunk.size();
106-
aliased.node->next = chunks_[chunk_ptr];
107-
chunks_[chunk_ptr] = aliased.node;
99+
FreeListNode *node =
100+
::new (chunk.data()) FreeListNode{chunks_[chunk_ptr], chunk.size()};
101+
chunks_[chunk_ptr] = node;
108102

109103
return true;
110104
}
@@ -123,17 +117,13 @@ span<cpp::byte> FreeList<NUM_BUCKETS>::find_chunk(size_t size) const {
123117

124118
// Now iterate up the buckets, walking each list to find a good candidate
125119
for (size_t i = chunk_ptr; i < chunks_.size(); i++) {
126-
union {
127-
FreeListNode *node;
128-
cpp::byte *data;
129-
} aliased;
130-
aliased.node = chunks_[static_cast<unsigned short>(i)];
120+
FreeListNode *node = chunks_[static_cast<unsigned short>(i)];
131121

132-
while (aliased.node != nullptr) {
133-
if (aliased.node->size >= size)
134-
return span<cpp::byte>(aliased.data, aliased.node->size);
122+
while (node != nullptr) {
123+
if (node->size >= size)
124+
return span<cpp::byte>(reinterpret_cast<cpp::byte *>(node), node->size);
135125

136-
aliased.node = aliased.node->next;
126+
node = node->next;
137127
}
138128
}
139129

@@ -146,34 +136,27 @@ template <size_t NUM_BUCKETS>
146136
bool FreeList<NUM_BUCKETS>::remove_chunk(span<cpp::byte> chunk) {
147137
size_t chunk_ptr = find_chunk_ptr_for_size(chunk.size(), true);
148138

149-
// Walk that list, finding the chunk.
150-
union {
151-
FreeListNode *node;
152-
cpp::byte *data;
153-
} aliased, aliased_next;
154-
155139
// Check head first.
156140
if (chunks_[chunk_ptr] == nullptr)
157141
return false;
158142

159-
aliased.node = chunks_[chunk_ptr];
160-
if (aliased.data == chunk.data()) {
161-
chunks_[chunk_ptr] = aliased.node->next;
143+
FreeListNode *node = chunks_[chunk_ptr];
144+
if (reinterpret_cast<cpp::byte *>(node) == chunk.data()) {
145+
chunks_[chunk_ptr] = node->next;
162146
return true;
163147
}
164148

165149
// No? Walk the nodes.
166-
aliased.node = chunks_[chunk_ptr];
150+
node = chunks_[chunk_ptr];
167151

168-
while (aliased.node->next != nullptr) {
169-
aliased_next.node = aliased.node->next;
170-
if (aliased_next.data == chunk.data()) {
152+
while (node->next != nullptr) {
153+
if (reinterpret_cast<cpp::byte *>(node->next) == chunk.data()) {
171154
// Found it, remove this node out of the chain
172-
aliased.node->next = aliased_next.node->next;
155+
node->next = node->next->next;
173156
return true;
174157
}
175158

176-
aliased.node = aliased.node->next;
159+
node = node->next;
177160
}
178161

179162
return false;

0 commit comments

Comments
 (0)