Skip to content

Fix use-after-free in virtiofs request worker thread#40792

Open
OneBlue wants to merge 6 commits into
masterfrom
user/oneblue/virtiofs-thread
Open

Fix use-after-free in virtiofs request worker thread#40792
OneBlue wants to merge 6 commits into
masterfrom
user/oneblue/virtiofs-thread

Conversation

@OneBlue

@OneBlue OneBlue commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Summary of the Pull Request

This change solves a potential use-after-free caused by the lack of synchronization with threads that are spawned to handle virtiofs requests from the guest.

To solve this, this change moves that method to use overlapped IO and handle both the accepts() and the requests with a single thread

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 June 12, 2026 19:53

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 VirtioFS request handling path by moving request processing to an overlapped I/O model that can accept connections and process requests within a single worker thread. It also refactors shared accept logic to return an accepted socket directly, and adds a regression-style test to stress multiple VirtioFS-backed DrvFs mounts.

Changes:

  • Refactor VirtioFS request worker to use io::MultiHandleWait with overlapped accept/read/write handles instead of spawning a thread per request.
  • Change socket::CancellableAccept to return std::optional<wil::unique_socket> and introduce a reusable io::AcceptHandle implementation.
  • Add a DrvFs test that mounts many VirtioFS shares in a loop to validate stability.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/windows/DrvFsTests.cpp Adds a stress test that mounts many VirtioFS DrvFs shares and validates access/options.
src/windows/wslrelay/main.cpp Updates to new CancellableAccept return type (optional accepted socket).
src/windows/wslrelay/localhost.cpp Updates accept loop to new CancellableAccept return type and moves socket out of optional.
src/windows/service/exe/WslCoreVm.h Adds ProcessVirtioFsRequest helper declaration for the new VirtioFS worker flow.
src/windows/service/exe/WslCoreVm.cpp Reworks VirtioFS worker to single-threaded overlapped accept/read/write and factors request processing into ProcessVirtioFsRequest.
src/windows/common/socket.hpp Updates CancellableAccept signature to return an optional accepted socket.
src/windows/common/socket.cpp Implements new CancellableAccept using io::AcceptHandle and returns the accepted socket via std::optional.
src/windows/common/hvsocket.cpp Delegates HvSocket CancellableAccept to the updated generic socket accept helper.
src/windows/common/HandleIO.h Replaces SingleAcceptHandle with reusable AcceptHandle that can accept once or repeatedly and owns the accepted socket.
src/windows/common/HandleIO.cpp Implements AcceptHandle (socket creation, accept completion, accept-context update, cancellation handling).

Comment thread test/windows/DrvFsTests.cpp
Comment thread test/windows/DrvFsTests.cpp
Comment thread src/windows/common/HandleIO.cpp Outdated
@OneBlue OneBlue marked this pull request as ready for review June 12, 2026 22:59
@OneBlue OneBlue requested a review from a team as a code owner June 12, 2026 22:59
Copilot AI review requested due to automatic review settings June 12, 2026 22:59

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

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comment thread test/windows/DrvFsTests.cpp
Comment on lines +29 to 40
std::optional<wil::unique_socket> wsl::windows::common::socket::CancellableAccept(
_In_ SOCKET ListenSocket, _In_ DWORD Timeout, _In_opt_ HANDLE ExitHandle, _In_ const std::source_location& Location)
{
io::MultiHandleWait io;

bool accepted = false;
std::optional<wil::unique_socket> accepted;

io.AddHandle(std::make_unique<io::SingleAcceptHandle>(ListenSocket, Socket, [&]() { accepted = true; }), io::MultiHandleWait::CancelOnCompleted);
io.AddHandle(
std::make_unique<io::AcceptHandle>(
ListenSocket, true, [&accepted](wil::unique_socket&& socket) { accepted = std::move(socket); }),
io::MultiHandleWait::CancelOnCompleted);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants