Fix use-after-free in UDPSimSocket::bind() due to missing addRef#13172
Fix use-after-free in UDPSimSocket::bind() due to missing addRef#13172saintstack merged 1 commit intoapple:mainfrom
Conversation
The UDPSimSocket constructor stored a raw `this` pointer into process->boundUDPSockets via emplace(), which direct-initializes a Reference<IUDPSocket> from the raw pointer. The explicit Reference(P*) constructor does not call addref — it assumes ownership of the initial refcount. But that initial refcount already belongs to the Reference returned by createUDPSocket(). Result: two Reference objects sharing one refcount. When bind() erased the old map entry, the refcount dropped to 0 and freed the object. The next line then called addRef(this) on freed memory — a heap-use-after-free. This was a latent bug that only crashed when buggify altered memory allocation patterns enough for the freed memory to be reused between lines 2531 and 2532. Certain buggify seeds triggered it reliably, causing RandomUnitTests and other simulation tests to segfault. Found by building with AddressSanitizer (-fsanitize=address), which immediately reported the exact free/use sequence in UDPSimSocket::bind(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes a simulation-only heap-use-after-free in UDPSimSocket construction by ensuring the simulator’s boundUDPSockets map holds its own reference to the socket object (instead of implicitly “stealing” the initial refcount).
Changes:
- Replace
emplace(localAddress, this)with an explicitReference<IUDPSocket>::addRef(this)when inserting intoprocess->boundUDPSockets.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr on Linux RHEL 9
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
|
Odd one... |
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr on Linux RHEL 9
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
spraza
left a comment
There was a problem hiding this comment.
lgtm, thanks for fixing this
|
I just ran this for this PR standalone: |
The UDPSimSocket constructor stored a raw
thispointer into process->boundUDPSockets via emplace(), which direct-initializes a Reference from the raw pointer. The explicit Reference(P*) constructor does not call addref — it assumes ownership of the initial refcount. But that initial refcount already belongs to the Reference returned by createUDPSocket(). Result: two Reference objects sharing one refcount.When bind() erased the old map entry, the refcount dropped to 0 and freed the object. The next line then called addRef(this) on freed memory — a heap-use-after-free.
This was a latent bug that only crashed when buggify altered memory allocation patterns enough for the freed memory to be reused between lines 2531 and 2532. Certain buggify seeds triggered it reliably, causing RandomUnitTests and other simulation tests to segfault.
Found by building with AddressSanitizer (-fsanitize=address), which immediately reported the exact free/use sequence in UDPSimSocket::bind().
(Found by claude debugging simulation failures)