[libc-commits] [libc] [libc][stdlib] Fix UB in freelist (PR #95330)

via libc-commits libc-commits at lists.llvm.org
Thu Jun 13 12:35:55 PDT 2024


================
@@ -92,19 +93,14 @@ bool FreeList<NUM_BUCKETS>::add_chunk(span<cpp::byte> chunk) {
   if (chunk.size() < sizeof(FreeListNode))
     return false;
 
-  union {
-    FreeListNode *node;
-    cpp::byte *bytes;
-  } aliased;
-
-  aliased.bytes = chunk.data();
-
+  // Add it to the correct list.
   size_t chunk_ptr = find_chunk_ptr_for_size(chunk.size(), false);
 
-  // Add it to the correct list.
-  aliased.node->size = chunk.size();
-  aliased.node->next = chunks_[chunk_ptr];
-  chunks_[chunk_ptr] = aliased.node;
+  FreeListNode node;
+  node.next = chunks_[chunk_ptr];
+  node.size = chunk.size();
+  LIBC_NAMESPACE::memcpy(chunk.data(), &node, sizeof(node));
----------------
PiJoules wrote:

Hmm, how might I use it in this case? The idea here is to store the `FreListNode` onto the `chunk` buffer, but `bit_cast` returns an object rather than reference so I wouldn't be able to do something like:

```
FreeListNode &node = cpp::bit_cast<FreeListNode>(chunk.data());
node.next = chunks_[chunk_ptr];
node.size = chunk.size();
```

Syntax-wise, this works
```
FreeListNode *node = cpp::bit_cast<FreeListNode *>(chunk.data());
node->next = chunks_[chunk_ptr];
node->size = chunk.size();
```
but I think this would still be UB by breaking strict aliasing unless the original  `chunk.data()` points to an actual `FreeListNode`.

Now that I think about it, the approach I have below isn't appropriate also since we have
```
FreeListNode *node = chunks_[...];  // Read from chunks_
node->size ... // Access from node
```
but `chunks_` always points to a buffer of `cpp::byte` and never a `FreeListNode` object so any accesses to those are also UB. Lemme fixe those also. 

https://github.com/llvm/llvm-project/pull/95330


More information about the libc-commits mailing list