Skip to content

[release/2.7] Host plugin Plan9 shares as the session user - #41557

Merged
Ben Hillis (benhillis) merged 2 commits into
release/2.7from
backport/2.7/plugin-plan9-user-server
Sep 11, 2026
Merged

Ben Hillis (benhillis) merged 2 commits into
release/2.7from
backport/2.7/plugin-plan9-user-server

Conversation

@benhillis

Copy link
Copy Markdown
Member

Backport of #41548 to release/2.7.

Plugin folder mounts used the HCS-managed Plan9 server, which accesses host paths with the service identity. Host plugin shares in a dedicated per-user Plan9 server on a separate port instead, so the session user's Windows file permissions are enforced. Existing GPU shares are unchanged.

Conflicts: PluginTestType and the test plugin's range check (release/2.7 does not have the wslc test types); the expected TestMode value in the new test was updated to match.

* Host plugin Plan9 shares as the session user

Use a dedicated per-user Plan9 server for plugin folder mounts so host filesystem permissions are preserved without relying on HCS-managed share identity.

Co-authored-by: Copilot <[email protected]>

Copilot-Session: c20b1da0-c613-489d-92a3-9c27527c0c54

* Simplify plugin Plan9 port plumbing

Use the fixed plugin port directly in mini_init and mirror the existing per-user Plan9 server lifecycle.

Co-authored-by: Copilot <[email protected]>

Copilot-Session: c20b1da0-c613-489d-92a3-9c27527c0c54

* Recreate stopped plugin Plan9 servers

Recreate the per-user server before adding a share when its process is no longer running.

Co-authored-by: Copilot <[email protected]>

Copilot-Session: c20b1da0-c613-489d-92a3-9c27527c0c54

---------

Co-authored-by: Ben Hillis <[email protected]>
Copilot-Session: c20b1da0-c613-489d-92a3-9c27527c0c54
(cherry picked from commit de25862)
Copilot AI lite review requested due to automatic review settings September 10, 2026 17:40
@benhillis
Ben Hillis (benhillis) requested a review from a team as a code owner September 10, 2026 17:40

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.

🟡 Changes recommended

The new Plan9 server lifecycle handling has a concrete teardown/locking issue that can lead to leaking/unsafe replacement of the COM server instance.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This backport changes how host plugin folder mounts are serviced so that host paths are accessed using the session user’s Windows permissions (instead of the service identity), by introducing a dedicated per-user Plan9 server on a separate port and updating mini_init mounting accordingly.

Changes:

  • Create and reuse a per-session-user Plan9 server for plugin folder shares and mount them through a new dedicated Plan9 port.
  • Extend mini_init’s Plan9 mounting helper to connect to a specified host port, and route plugin “mount folder” messages to the plugin Plan9 port.
  • Add a regression test that validates allowed vs denied writes through a plugin-mounted folder.
File summaries
File Description
test/windows/testplugin/Plugin.cpp Adds a MountFolderAccess test mode that mounts a host folder and attempts allowed/denied writes from Linux.
test/windows/PluginTests.h Adds MountFolderAccess test type and a registry value name for passing the mount folder path.
test/windows/PluginTests.cpp Extends plugin configuration to pass mount folder path and adds MountFolderAccess regression test (ACL deny write).
src/windows/service/exe/WslCoreVm.h Adds a cached m_pluginPlan9Server member for plugin shares.
src/windows/service/exe/WslCoreVm.cpp Implements per-user Plan9 server creation/reuse for plugin mounts and teardown in destructor.
src/shared/inc/lxinitshared.h Introduces LX_INIT_UTILITY_VM_PLAN9_PLUGIN_PORT (50006).
src/linux/init/main.cpp Updates MountPlan9 to accept a host port and mounts plugin folders via the plugin Plan9 port (GPU shares unchanged).
Review details

Suppressed comments (1)

src/windows/service/exe/WslCoreVm.cpp:2075

  • If m_pluginPlan9Server exists but IsRunning() is not S_OK, the code creates a new server and overwrites m_pluginPlan9Server without explicitly tearing down the old instance. If the old server still holds resources (e.g., bound port / shares), this can leak and/or make reinitialization flaky. Teardown/reset the existing server before creating the replacement.
        if (!m_pluginPlan9Server || m_pluginPlan9Server->IsRunning() != S_OK)
        {
            auto server =
                wsl::windows::common::wslutil::CreateComServerAsUser<p9fs::Plan9FileSystem, IPlan9FileSystem>(m_userToken.get());
            THROW_IF_FAILED(server->Init(&m_runtimeId, LX_INIT_UTILITY_VM_PLAN9_PLUGIN_PORT));
            THROW_IF_FAILED(server->Resume());
            m_pluginPlan9Server = std::move(server);
        }
  • Files reviewed: 7/7 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/windows/service/exe/WslCoreVm.cpp
When the per-user plugin Plan9 server is no longer running, the previous instance was dropped without a Teardown call, so it could still hold the Plan9 port when the replacement tries to bind it.

Co-authored-by: Copilot <[email protected]>
Copilot-Session: 0d028ad4-fe5e-4ef1-9832-18ee0e4825cc
(cherry picked from commit c247a84)
Copilot AI review requested due to automatic review settings September 10, 2026 22:44

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.

🔵 Needs a closer look

A moderate ERROR_ALREADY_EXISTS handling issue in WslCoreVm.cpp remains unresolved.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/windows/service/exe/WslCoreVm.cpp:2084

  • This path now propagates ERROR_ALREADY_EXISTS from AddSharePath, whereas the existing Plan9-share path treats that result as success (see WslCoreVm.cpp:960-966). A plugin that retries a mount after a partial mount, or mounts two folders with the same share name, will therefore fail before mini_init is asked to mount it; normalize this result to S_OK here as well.
  • Files reviewed: 7/7 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@benhillis
Ben Hillis (benhillis) enabled auto-merge (squash) September 10, 2026 23:53
@benhillis
Ben Hillis (benhillis) merged commit 161ba49 into release/2.7 Sep 11, 2026
8 checks passed
@benhillis
Ben Hillis (benhillis) deleted the backport/2.7/plugin-plan9-user-server branch September 11, 2026 00:31
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.

3 participants