Repository navigation
[release/2.7] Host plugin Plan9 shares as the session user - #41557
Conversation
* 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)
There was a problem hiding this comment.
🟡 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_pluginPlan9Serverexists butIsRunning()is notS_OK, the code creates a new server and overwritesm_pluginPlan9Serverwithout 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.
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)
There was a problem hiding this comment.
🔵 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_EXISTSfromAddSharePath, whereas the existing Plan9-share path treats that result as success (seeWslCoreVm.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 toS_OKhere as well.
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
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:
PluginTestTypeand the test plugin's range check (release/2.7 does not have the wslc test types); the expectedTestModevalue in the new test was updated to match.