Skip to content

fix(usockets): safely handle socket reallocation during context adoption #25361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
cirospaciari wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

fix(usockets): safely handle socket reallocation during context adoption #25361

cirospaciari wants to merge 2 commits into from
+141 −59

Conversation

@cirospaciari
Copy link
Member

@cirospaciari cirospaciari commented Dec 5, 2025 •
edited
Loading

Summary

  • Fix use-after-free vulnerability during socket adoption by properly tracking reallocated sockets
  • Add safety checks to prevent linking closed sockets to context lists
  • Properly track socket state with new is_closed, adopted, and is_tls flags

What does this PR do?

This PR improves event loop stability by addressing potential use-after-free issues that can occur when sockets are reallocated during adoption (e.g., when upgrading a TCP socket to TLS).

Key Changes

Socket State Tracking (internal.h)

  • Added is_closed flag to explicitly track when a socket has been closed
  • Added adopted flag to mark sockets that were reallocated during context adoption
  • Added is_tls flag to track TLS socket state for proper low-priority queue handling

Safe Socket Adoption (context.c)

  • When us_poll_resize() returns a new pointer (reallocation occurred), the old socket is now:
    • Marked as closed (is_closed = 1)
    • Added to the closed socket cleanup list
    • Marked as adopted (adopted = 1)
    • Has its prev pointer set to the new socket for event redirection
  • Added guards to us_internal_socket_context_link_socket/listen_socket/connecting_socket to prevent linking already-closed sockets

Event Loop Handling (loop.c)

  • After callbacks that can trigger socket adoption (on_open, on_writable, on_data), the event loop now checks if the socket was reallocated and redirects to the new socket
  • Low-priority socket handling now properly checks is_closed state and uses is_tls flag for correct SSL handling

Poll Resize Safety (epoll_kqueue.c)

  • Changed us_poll_resize() to always allocate new memory with us_calloc() instead of us_realloc() to ensure the old pointer remains valid for cleanup
  • Now takes old_ext_size parameter to correctly calculate memory sizes
  • Re-enabled us_internal_loop_update_pending_ready_polls() call in us_poll_change() to ensure pending events are properly redirected

How did you verify your code works?

Run existing CI and existing socket upgrade tests under asan build

@robobun
Copy link
Collaborator

robobun commented Dec 5, 2025 •
edited
Loading

Updated 2:51 PM PT - Dec 5th, 2025

❌ @cirospaciari, your commit b2fbbd2 has 45 failures in Build #32919 (All Failures):

🧪   To try this PR locally:

bunx bun-pr 25361

That installs a local version of the PR into your bun-25361 executable, so you can run:

bun-25361 --bun

@cirospaciari cirospaciari changed the title WIP fix(usockets): safely handle socket reallocation during context adoption Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cirospaciari @robobun