Skip to content

Commit c13fe64

Browse files
Ben HillisCopilot
andcommitted
Isolate plugins in an out-of-process COM host
WSL plugin DLLs are moved out of wslservice.exe into a separate wslpluginhost.exe COM server so plugin code can no longer crash or destabilize the service. Each plugin is activated in its own host process (CLSCTX_LOCAL_SERVER, SYSTEM-only via AppID) and reached through a versioned COM interface defined in WslPluginHost.idl. All hosts are tied to a service-owned job object and terminate when wslservice exits. The plugin API is unchanged; existing plugins run unmodified. A crashing or disconnected host is classified by IsHostCrash (RPC_E_DISCONNECTED, RPC_E_SERVER_DIED[_DNE], CO_E_OBJNOTCONNECTED, RPC_S_SERVER_UNAVAILABLE, RPC_S_CALL_FAILED[_DNE]); the service logs it and continues instead of treating it as a fatal plugin error. RPC_E_CALL_REJECTED is intentionally excluded as a transient busy state rather than a dead host. Plugin->service callbacks (MountFolder, ExecuteBinary, and the WSLC session APIs) arrive on a different COM thread than the outbound hook, so they cannot re-enter the lock held during the hook: - VM path: LxssUserSessionImpl guards callbacks with a shared_mutex (shared for callbacks, exclusive in _VmTerminate after OnVmStopping drains in-flight callbacks before the utility VM is destroyed). - WSLC path: PluginManager resolves sessions through its own reference map under a dedicated lock, and WSLCSessionManager releases its session lock before any plugin notification fires, so callbacks never re-enter the session lock. A session is registered in the reference map but not published until OnWslcSessionCreated succeeds, so a vetoed or race-lost session is never handed out. Proxy/stub is consolidated into wslserviceproxystub.dll. One new exe, no new DLLs. Tests - HostCrashIsolation: kills wslpluginhost.exe mid-OnVmStarted and verifies the service survives and m_initOnce stays sticky. - ConcurrentCallbacks: four plugin threads hammer MountFolder and ExecuteBinary, exercising the shared callback lock. - AsyncApiCallFromWorker: a plugin worker thread calls into the service post-hook (cross-apartment, non-COM-initialized). - CallbacksDuringTerminationDoNotCrash: worker threads race _VmTerminate's exclusive lock and VM teardown, then wind down deterministically after OnVmStopping signals them and are joined on the next session start. - Existing WSL1 plugin tests broadened alongside the refactor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent e0198f1 commit c13fe64

30 files changed

Lines changed: 4040 additions & 688 deletions

.pipelines/build-stage.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ parameters:
3232
- name: targets
3333
type: object
3434
default:
35-
- target: "wsl;libwsl;wslg;wslservice;wslhost;wslrelay;wslinstaller;wslinstall;initramfs;wslserviceproxystub;wslsettings;wslinstallerproxystub;testplugin;wslcsession;wslc;wsltests;wslcsdk"
36-
pattern: "wsl.exe,libwsl.dll,wslg.exe,wslservice.exe,wslhost.exe,wslrelay.exe,wslinstaller.exe,wslinstall.dll,wslserviceproxystub.dll,wslsettings/wslsettings.dll,wslsettings/wslsettings.exe,wslinstallerproxystub.dll,WSLDVCPlugin.dll,testplugin.dll,wsldeps.dll,wslcsession.exe,wslc.exe,wslcsdk.dll"
35+
- target: "wsl;libwsl;wslg;wslservice;wslhost;wslrelay;wslpluginhost;wslinstaller;wslinstall;initramfs;wslserviceproxystub;wslsettings;wslinstallerproxystub;testplugin;wslcsession;wslc;wsltests;wslcsdk"
36+
pattern: "wsl.exe,libwsl.dll,wslg.exe,wslservice.exe,wslhost.exe,wslrelay.exe,wslpluginhost.exe,wslinstaller.exe,wslinstall.dll,wslserviceproxystub.dll,wslsettings/wslsettings.dll,wslsettings/wslsettings.exe,wslinstallerproxystub.dll,WSLDVCPlugin.dll,testplugin.dll,wsldeps.dll,wslcsession.exe,wslc.exe,wslcsdk.dll"
3737
- target: "msixgluepackage"
3838
pattern: "gluepackage.msix"
3939
- target: "msipackage;wslcsdkcs"

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ add_subdirectory(src/windows/wsl)
535535
add_subdirectory(src/windows/wslg)
536536
add_subdirectory(src/windows/wslhost)
537537
add_subdirectory(src/windows/wslrelay)
538+
add_subdirectory(src/windows/wslpluginhost)
538539
add_subdirectory(src/windows/wslinstall)
539540
add_subdirectory(src/windows/wslc)
540541
add_subdirectory(src/windows/WslcSDK)

msipackage/CMakeLists.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ set(OUTPUT_PACKAGE ${BIN}/wsl.msi)
1717
set(PACKAGE_WIX_IN ${CMAKE_CURRENT_LIST_DIR}/package.wix.in)
1818
set(PACKAGE_WIX ${BIN}/package.wix)
1919
set(CAB_CACHE ${BIN}/cab)
20-
set(WINDOWS_BINARIES wsl.exe;wslg.exe;wslhost.exe;wslrelay.exe;wslservice.exe;wslserviceproxystub.dll;wslinstall.dll;wslc.exe;wslcsession.exe)
20+
set(WINDOWS_BINARIES wsl.exe;wslg.exe;wslhost.exe;wslrelay.exe;wslpluginhost.exe;wslservice.exe;wslserviceproxystub.dll;wslinstall.dll;wslc.exe;wslcsession.exe)
2121
if (WSL_BUILD_WSL_SETTINGS)
2222
list(APPEND WINDOWS_BINARIES "wslsettings/wslsettings.dll;wslsettings/wslsettings.exe;libwsl.dll")
2323
endif()
@@ -57,7 +57,7 @@ add_custom_command(
5757

5858
add_custom_target(msipackage DEPENDS ${OUTPUT_PACKAGE})
5959
set_target_properties(msipackage PROPERTIES EXCLUDE_FROM_ALL FALSE SOURCES ${PACKAGE_WIX_IN})
60-
add_dependencies(msipackage wsl wslg wslservice wslhost wslrelay wslserviceproxystub init initramfs wslinstall msixgluepackage wslc wslcsession)
60+
add_dependencies(msipackage wsl wslg wslservice wslhost wslrelay wslpluginhost wslserviceproxystub init initramfs wslinstall msixgluepackage wslc wslcsession)
6161

6262
if (WSL_BUILD_WSL_SETTINGS)
6363
add_dependencies(msipackage wslsettings libwsl)

msipackage/package.wix.in

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
<File Id="container.exe" Name="container.exe" Source="${PACKAGE_INPUT_DIR}/wslc.exe" />
3232
<File Id="wslhost.exe" Name="wslhost.exe" Source="${PACKAGE_INPUT_DIR}/wslhost.exe" />
3333
<File Id="wslrelay.exe" Name="wslrelay.exe" Source="${PACKAGE_INPUT_DIR}/wslrelay.exe" />
34+
<File Id="wslpluginhost.exe" Name="wslpluginhost.exe" Source="${PACKAGE_INPUT_DIR}/wslpluginhost.exe" />
3435
<File Id="wslserviceproxystub.dll" Name="wslserviceproxystub.dll" Source="${PACKAGE_INPUT_DIR}/wslserviceproxystub.dll" />
3536
<File Id="wsldeps.dll" Name="wsldeps.dll" Source="${PACKAGE_INPUT_DIR}/wsldeps.dll" />
3637

@@ -177,6 +178,35 @@
177178
<RegistryValue Value='"[INSTALLDIR]wslhost.exe"' Type="string" />
178179
</RegistryKey>
179180

181+
<!-- WslPluginHost - out-of-process plugin isolation (process spawned by service) -->
182+
<!-- WslPluginHost AppID — SYSTEM-only activation -->
183+
<!-- O:SYG:SYD:(A;;CCDCSW;;;SY) -->
184+
<RegistryKey Root="HKCR" Key="AppID\{7a1d2c3e-4b5f-6a7d-8e9f-0a1b2c3d4e5f}">
185+
<RegistryValue Name="AccessPermission" Value="010004801400000020000000000000002C00000001010000000000051200000001010000000000051200000002001C0001000000000014000B000000010100000000000512000000" Type="binary" />
186+
<RegistryValue Name="LaunchPermission" Value="010004801400000020000000000000002C00000001010000000000051200000001010000000000051200000002001C0001000000000014000B000000010100000000000512000000" Type="binary" />
187+
</RegistryKey>
188+
<RegistryKey Root="HKCR" Key="CLSID\{7a1d2c3e-4b5f-6a7d-8e9f-0a1b2c3d4e5f}">
189+
<RegistryValue Name="AppId" Value="{7a1d2c3e-4b5f-6a7d-8e9f-0a1b2c3d4e5f}" Type="string" />
190+
<RegistryValue Value="WslPluginHost" Type="string" />
191+
</RegistryKey>
192+
<RegistryKey Root="HKCR" Key="CLSID\{7a1d2c3e-4b5f-6a7d-8e9f-0a1b2c3d4e5f}\LocalServer32">
193+
<RegistryValue Value='"[INSTALLDIR]wslpluginhost.exe"' Type="string" />
194+
</RegistryKey>
195+
196+
<!-- WslPluginHost interfaces — use the shared wslserviceproxystub.dll -->
197+
<RegistryKey Root="HKCR" Key="Interface\{A2B3C4D5-E6F7-4890-AB12-CD34EF56A789}">
198+
<RegistryValue Value="IWslPluginHostCallback" Type="string" />
199+
<RegistryKey Key="ProxyStubClsid32">
200+
<RegistryValue Value="{4EA0C6DD-E9FF-48E7-994E-13A31D10DC60}" Type="string" />
201+
</RegistryKey>
202+
</RegistryKey>
203+
<RegistryKey Root="HKCR" Key="Interface\{B3C4D5E6-F7A8-4901-BC23-DE45FA67B890}">
204+
<RegistryValue Value="IWslPluginHost" Type="string" />
205+
<RegistryKey Key="ProxyStubClsid32">
206+
<RegistryValue Value="{4EA0C6DD-E9FF-48E7-994E-13A31D10DC60}" Type="string" />
207+
</RegistryKey>
208+
</RegistryKey>
209+
180210
<!-- wsldevicehost.dll -->
181211
<RegistryKey Root="HKCR" Key="AppID\{17696EAC-9568-4CF5-BB8C-82515AAD6C09}">
182212
<RegistryValue Name="DllSurrogate" Value="" Type="string" />

src/windows/common/precomp.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ Module Name:
8484
#include <optional>
8585
#include <filesystem>
8686
#include <mutex>
87+
#include <shared_mutex>
8788
#include <chrono>
8889
#include <codecvt>
8990
#include <random>

src/windows/service/exe/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ set(HEADERS
5858
WSLCPluginNotifier.h)
5959

6060
add_executable(wslservice ${SOURCES} ${HEADERS})
61-
add_dependencies(wslservice wslserviceidl wslservicemc)
61+
add_dependencies(wslservice wslserviceidl wslservicemc wslpluginhostidl)
6262
add_compile_definitions(__WRL_CLASSIC_COM__)
6363
add_compile_definitions(__WRL_DISABLE_STATIC_INITIALIZE__)
6464
add_compile_definitions(USE_COM_CONTEXT_DEF=1)

src/windows/service/exe/LxssUserSession.cpp

Lines changed: 55 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2631,13 +2631,18 @@ std::shared_ptr<LxssRunningInstance> LxssUserSessionImpl::_CreateInstance(_In_op
26312631
registration.Write(Property::OsVersion, distributionInfo->Version);
26322632
}
26332633

2634-
// This needs to be done before plugins are notifed because they might try to run a command inside the distribution.
2635-
m_runningInstances[registration.Id()] = instance;
2634+
// This needs to be done before plugins are notified because they might try to run a command inside the distribution.
2635+
{
2636+
std::unique_lock callbackLock(m_callbackLock);
2637+
m_runningInstances[registration.Id()] = instance;
2638+
}
26362639

26372640
if (version == LXSS_WSL_VERSION_2)
26382641
{
2639-
auto cleanupOnFailure =
2640-
wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { m_runningInstances.erase(registration.Id()); });
2642+
auto cleanupOnFailure = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() {
2643+
std::unique_lock callbackLock(m_callbackLock);
2644+
m_runningInstances.erase(registration.Id());
2645+
});
26412646
m_pluginManager.OnDistributionStarted(&m_session, instance->DistributionInformation());
26422647
cleanupOnFailure.release();
26432648
}
@@ -2873,7 +2878,13 @@ void LxssUserSessionImpl::_CreateVm()
28732878
m_vmId.store(vmId);
28742879

28752880
// Create the utility VM and register for callbacks.
2876-
m_utilityVm = WslCoreVm::Create(m_userToken, std::move(config), vmId);
2881+
// Publish m_utilityVm under m_callbackLock exclusive to honor the dual-lock
2882+
// invariant for mutations of m_utilityVm; this is uncontended here because
2883+
// no plugin callbacks can race against initial creation.
2884+
{
2885+
std::unique_lock callbackLock(m_callbackLock);
2886+
m_utilityVm = WslCoreVm::Create(m_userToken, std::move(config), vmId);
2887+
}
28772888

28782889
if (m_httpProxyStateTracker)
28792890
{
@@ -3604,17 +3615,26 @@ bool LxssUserSessionImpl::_TerminateInstanceInternal(_In_ LPCGUID DistroGuid, _I
36043615
m_pluginManager.OnDistributionStopping(&m_session, wslcoreInstance->DistributionInformation());
36053616
}
36063617

3607-
instance->second->Stop();
3618+
m_lifetimeManager.RemoveCallback(clientKey);
36083619

3609-
const auto clientId = instance->second->GetClientId();
3620+
// Stop the instance and remove it from m_runningInstances atomically
3621+
// under m_callbackLock. This prevents plugin callbacks (which hold
3622+
// m_callbackLock shared) from finding a stopped-but-still-listed
3623+
// instance between Stop() and erase.
3624+
ULONG clientId;
36103625
{
3611-
auto lock = m_terminatedInstanceLock.lock_exclusive();
3612-
m_terminatedInstances.push_back(std::move(instance->second));
3613-
}
3626+
std::unique_lock callbackLock(m_callbackLock);
36143627

3615-
m_lifetimeManager.RemoveCallback(clientKey);
3628+
instance->second->Stop();
3629+
clientId = instance->second->GetClientId();
3630+
3631+
{
3632+
auto lock = m_terminatedInstanceLock.lock_exclusive();
3633+
m_terminatedInstances.push_back(std::move(instance->second));
3634+
}
36163635

3617-
m_runningInstances.erase(instance);
3636+
m_runningInstances.erase(instance);
3637+
}
36183638

36193639
// If the instance that was terminated was a WSL2 instance,
36203640
// check if the VM is now idle.
@@ -3642,7 +3662,10 @@ void LxssUserSessionImpl::_UpdateInit(_In_ const LXSS_DISTRO_CONFIGURATION& Conf
36423662

36433663
HRESULT LxssUserSessionImpl::MountRootNamespaceFolder(_In_ LPCWSTR HostPath, _In_ LPCWSTR GuestPath, _In_ bool ReadOnly, _In_ LPCWSTR Name)
36443664
{
3645-
std::lock_guard lock(m_instanceLock);
3665+
// Shared lock prevents _VmTerminate from destroying the VM while we use it.
3666+
// Do NOT acquire m_instanceLock — callbacks arrive on a different COM thread
3667+
// from the notification thread that holds m_instanceLock.
3668+
std::shared_lock lock(m_callbackLock);
36463669
RETURN_HR_IF(E_NOT_VALID_STATE, !m_utilityVm);
36473670

36483671
m_utilityVm->MountRootNamespaceFolder(HostPath, GuestPath, ReadOnly, Name);
@@ -3651,7 +3674,9 @@ HRESULT LxssUserSessionImpl::MountRootNamespaceFolder(_In_ LPCWSTR HostPath, _In
36513674

36523675
HRESULT LxssUserSessionImpl::CreateLinuxProcess(_In_opt_ const GUID* Distro, _In_ LPCSTR Path, _In_ LPCSTR* Arguments, _Out_ SOCKET* Socket)
36533676
{
3654-
std::lock_guard lock(m_instanceLock);
3677+
// Shared lock prevents _VmTerminate from destroying the VM or instances
3678+
// while we use them. See MountRootNamespaceFolder for rationale.
3679+
std::shared_lock lock(m_callbackLock);
36553680
RETURN_HR_IF(E_NOT_VALID_STATE, !m_utilityVm);
36563681

36573682
if (Distro == nullptr)
@@ -3660,9 +3685,16 @@ HRESULT LxssUserSessionImpl::CreateLinuxProcess(_In_opt_ const GUID* Distro, _In
36603685
}
36613686
else
36623687
{
3663-
const auto distro = _RunningInstance(Distro);
3664-
THROW_HR_IF(WSL_E_VM_MODE_INVALID_STATE, !distro);
3665-
3688+
// Look up the running instance directly instead of calling _RunningInstance,
3689+
// which accesses m_lockedDistributions (guarded only by m_instanceLock).
3690+
// m_runningInstances is safe to read under m_callbackLock (shared).
3691+
// The _EnsureNotLocked check is unnecessary here: _ConversionBegin removes
3692+
// a distribution from m_runningInstances before adding it to m_lockedDistributions,
3693+
// so a locked distribution will never be found in this lookup.
3694+
const auto instance = m_runningInstances.find(*Distro);
3695+
THROW_HR_IF(WSL_E_VM_MODE_INVALID_STATE, instance == m_runningInstances.end());
3696+
3697+
const auto distro = instance->second;
36663698
const auto wsl2Distro = dynamic_cast<WslCoreInstance*>(distro.get());
36673699
THROW_HR_IF(WSL_E_WSL2_NEEDED, !wsl2Distro);
36683700

@@ -3900,7 +3932,12 @@ void LxssUserSessionImpl::_VmTerminate()
39003932
m_telemetryThread.join();
39013933
}
39023934

3903-
m_utilityVm.reset();
3935+
// Acquire exclusive callback lock to wait for any in-flight plugin callbacks
3936+
// (MountRootNamespaceFolder, CreateLinuxProcess) to complete before destroying the VM.
3937+
{
3938+
std::unique_lock callbackLock(m_callbackLock);
3939+
m_utilityVm.reset();
3940+
}
39043941
m_vmId.store(GUID_NULL);
39053942

39063943
// Reset the user's token since its lifetime is tied to the VM.

src/windows/service/exe/LxssUserSession.h

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,10 @@ class DECLSPEC_UUID("a9b7a1b9-0671-405c-95f1-e0612cb4ce7e") LxssUserSession
310310
/// </summary>
311311
class LxssUserSessionImpl
312312
{
313+
// Plugin callbacks arrive on a different COM RPC thread and use m_callbackLock
314+
// (shared) instead of m_instanceLock to access m_utilityVm and m_runningInstances.
315+
friend class wsl::windows::service::PluginHostCallbackImpl;
316+
313317
public:
314318
LxssUserSessionImpl(_In_ PSID userSid, _In_ DWORD sessionId, _Inout_ wsl::windows::service::PluginManager& pluginManager);
315319
virtual ~LxssUserSessionImpl();
@@ -363,11 +367,6 @@ class LxssUserSessionImpl
363367
/// </summary>
364368
void ClearDiskStateInRegistry(_In_opt_ LPCWSTR Disk);
365369

366-
/// <summary>
367-
/// Start a process in the root namespace or in a user distribution.
368-
/// </summary>
369-
HRESULT CreateLinuxProcess(_In_opt_ const GUID* Distro, _In_ LPCSTR Path, _In_ LPCSTR* Arguments, _Out_ SOCKET* socket);
370-
371370
/// <summary>
372371
/// Enumerates registered distributions, optionally including ones that are
373372
/// currently being registered, unregistered, or converted.
@@ -443,8 +442,6 @@ class LxssUserSessionImpl
443442

444443
HRESULT MoveDistribution(_In_ LPCGUID DistroGuid, _In_ LPCWSTR Location);
445444

446-
HRESULT MountRootNamespaceFolder(_In_ LPCWSTR HostPath, _In_ LPCWSTR GuestPath, _In_ bool ReadOnly, _In_ LPCWSTR Name);
447-
448445
/// <summary>
449446
/// Registers a distribution.
450447
/// </summary>
@@ -533,6 +530,18 @@ class LxssUserSessionImpl
533530
static CreateLxProcessContext s_GetCreateProcessContext(_In_ const GUID& DistroGuid, _In_ bool SystemDistro);
534531

535532
private:
533+
/// <summary>
534+
/// Plugin callback methods — called from PluginHostCallbackImpl on a COM RPC
535+
/// thread during plugin notifications. These acquire m_callbackLock (shared)
536+
/// instead of m_instanceLock, preventing _VmTerminate from destroying the VM
537+
/// while a callback is in-flight. Access is restricted via friend declaration.
538+
/// </summary>
539+
_Requires_lock_not_held_(m_instanceLock)
540+
HRESULT MountRootNamespaceFolder(_In_ LPCWSTR HostPath, _In_ LPCWSTR GuestPath, _In_ bool ReadOnly, _In_ LPCWSTR Name);
541+
542+
_Requires_lock_not_held_(m_instanceLock)
543+
HRESULT CreateLinuxProcess(_In_opt_ const GUID* Distro, _In_ LPCSTR Path, _In_ LPCSTR* Arguments, _Out_ SOCKET* socket);
544+
536545
/// <summary>
537546
/// Adds a distro to the list of converting distros.
538547
/// </summary>
@@ -794,7 +803,9 @@ class LxssUserSessionImpl
794803
std::recursive_timed_mutex m_instanceLock;
795804

796805
/// <summary>
797-
/// Contains the currently running utility VM's.
806+
/// Contains the currently running instances.
807+
/// Reads guarded by m_instanceLock OR m_callbackLock (shared).
808+
/// Mutations require BOTH m_instanceLock AND m_callbackLock (exclusive).
798809
/// </summary>
799810
_Guarded_by_(m_instanceLock) std::map<GUID, std::shared_ptr<LxssRunningInstance>, wsl::windows::common::helpers::GuidLess> m_runningInstances;
800811

@@ -811,9 +822,24 @@ class LxssUserSessionImpl
811822

812823
/// <summary>
813824
/// The running utility vm for WSL2 distributions.
814-
///
825+
/// Reads guarded by m_instanceLock OR m_callbackLock (shared).
826+
/// Mutations require BOTH m_instanceLock AND m_callbackLock (exclusive).
827+
/// </summary>
815828
_Guarded_by_(m_instanceLock) std::unique_ptr<WslCoreVm> m_utilityVm;
816829

830+
/// <summary>
831+
/// Reader-writer lock protecting m_utilityVm and m_runningInstances for
832+
/// plugin callbacks. Callbacks take a shared (read) lock; _VmTerminate and
833+
/// instance mutations take an exclusive (write) lock.
834+
///
835+
/// Mutations of m_runningInstances/m_utilityVm require BOTH m_instanceLock
836+
/// AND m_callbackLock (exclusive). Reads are safe under either lock alone.
837+
///
838+
/// Lock ordering: m_instanceLock → m_callbackLock (never reverse).
839+
/// Callbacks must NEVER acquire m_instanceLock (deadlock with notification thread).
840+
/// </summary>
841+
std::shared_mutex m_callbackLock;
842+
817843
std::atomic<GUID> m_vmId{GUID_NULL};
818844

819845
/// <summary>

0 commit comments

Comments
 (0)