Skip to content

Fix potential use-after-free on the container deletion path - #41018

Merged
Blue (OneBlue) merged 4 commits into
masterfrom
user/oneblue/fix-uaf
Jul 8, 2026
Merged

Blue (OneBlue) merged 4 commits into
masterfrom
user/oneblue/fix-uaf

Conversation

@OneBlue

Copy link
Copy Markdown
Collaborator

Summary of the Pull Request

There are two ways a container can be removed from m_containers:

  1. The container transitions to the Deleted state, and a later call that enumerates m_containers deletes it

  2. The callback from WSLCContainer::Delete()

If 1) happens while 2) is in progress, the callback from 2) will operate on a pointer that's been deleted.

This change solves the issue by switching 2) to use a std::weak_ptr instead of a raw pointer, which guarantees that the pointer will be valid until released.

This change also adds a stress test to protect from regressions on this path.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Copilot AI review requested due to automatic review settings July 7, 2026 18:05
@OneBlue
Blue (OneBlue) requested a review from a team as a code owner July 7, 2026 18:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a potential use-after-free in the wslc container deletion path by changing container lifetime management to shared ownership and by having the COM wrapper hold only a weak reference to the implementation. It also adds a stress test intended to exercise concurrent Delete() and ListContainers() interactions.

Changes:

  • Switch WSLCSession::m_containers storage from std::unique_ptr to std::shared_ptr and make WSLCContainerImpl enable_shared_from_this.
  • Update the COM forwarding helper (COMImplClass) and WSLCContainer wrapper to work with std::weak_ptr to avoid callbacks operating on freed memory.
  • Add ContainerListDeleteStressTest to stress concurrent list/delete behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/windows/WSLCTests.cpp Adds a concurrent create/delete + list stress test for regression coverage.
src/windows/wslcsession/WSLCSession.h Changes container map ownership to shared_ptr to support safe concurrent access.
src/windows/wslcsession/WSLCSession.cpp Makes OnContainerDeleted tolerant of the container already being removed.
src/windows/wslcsession/WSLCContainer.h Makes WSLCContainerImpl enable_shared_from_this and updates factory APIs + COM wrapper pointer type.
src/windows/wslcsession/WSLCContainer.cpp Constructs containers with make_shared, adds post-ctor initialization for weak pointer wiring, and adjusts delete callback usage.
src/windows/service/exe/WSLCSessionManager.cpp Updates initialization to match the new COMImplClass API.
src/windows/common/COMImplClass.h Generalizes forwarding helper to support weak-pointer-backed impl access.
Comment thread src/windows/common/COMImplClass.h
Comment thread test/windows/WSLCTests.cpp Outdated
Comment thread test/windows/WSLCTests.cpp Outdated
Comment thread test/windows/WSLCTests.cpp
@OneBlue
Blue (OneBlue) merged commit 7728814 into master Jul 8, 2026
11 checks passed
@OneBlue
Blue (OneBlue) deleted the user/oneblue/fix-uaf branch July 8, 2026 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants