Skip to content

Commit 77c3483

Browse files
author
Ben Hillis
committed
Isolate plugins in out-of-process COM host
Plugin DLLs are now loaded in isolated wslpluginhost.exe processes instead of directly in wslservice.exe via LoadLibrary. This prevents a buggy or malicious plugin from crashing the WSL service. Architecture: - New IWslPluginHost/IWslPluginHostCallback COM interfaces (WslPluginHost.idl) for cross-process plugin lifecycle management - New wslpluginhost.exe: COM local server (REGCLS_SINGLEUSE), one per plugin, loads the plugin DLL and dispatches notifications - Refactored PluginManager: CoCreateInstance replaces LoadLibrary, PluginError returned via [out] parameter, crash recovery via IsHostCrash() detecting RPC_E_DISCONNECTED/SERVER_DIED Callback safety: - Plugin callbacks (MountFolder, ExecuteBinary) arrive on a different COM RPC thread and use std::shared_lock(m_callbackLock) instead of m_instanceLock to avoid re-entrancy deadlocks - _VmTerminate takes exclusive m_callbackLock before destroying the VM, blocking until in-flight callbacks complete - Lock ordering: m_instanceLock -> m_callbackLock (never reverse) - All writes to m_runningInstances take m_callbackLock exclusive to prevent data races with concurrent callback reads Security: - COM AppID with SYSTEM-only launch/access permissions - Plugin signature validation (ValidateFileSignature) keeps the file handle open until after LoadLibrary to prevent TOCTOU attacks - Plugin host processes use minimal access rights for handles Process lifecycle: - Plugin hosts added to a job object with KILL_ON_JOB_CLOSE for automatic cleanup if wslservice exits - g_pluginHost is process-wide (REGCLS_SINGLEUSE guarantees one plugin per process), nulled on destruction to prevent UAF - std::call_once for thread-safe initialization and job creation Packaging: - WslPluginHost.idl compiled into existing wslserviceproxystub.dll - MSI: COM class/interface registration, AppID security, proxy/stub - wslpluginhost.exe added to build/signing pipeline, WER crash dump list, LSP registration, and test validation Plugins are not loaded for WSL1-only sessions since all plugin hooks require a WSL2 VM. WslPluginApi.h is unchanged - existing plugin DLLs work unmodified.
1 parent 813070c commit 77c3483

25 files changed

Lines changed: 1459 additions & 172 deletions

.pipelines/build-stage.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ parameters:
2727
- name: targets
2828
type: object
2929
default:
30-
- target: "wsl;libwsl;wslg;wslservice;wslhost;wslrelay;wslinstaller;wslinstall;initramfs;wslserviceproxystub;wslsettings;wslinstallerproxystub;testplugin"
31-
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"
30+
- target: "wsl;libwsl;wslg;wslservice;wslhost;wslrelay;wslpluginhost;wslinstaller;wslinstall;initramfs;wslserviceproxystub;wslsettings;wslinstallerproxystub;testplugin"
31+
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"
3232
- target: "msixgluepackage"
3333
pattern: "gluepackage.msix"
3434
- target: "msipackage"

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,7 @@ add_subdirectory(src/windows/wsl)
486486
add_subdirectory(src/windows/wslg)
487487
add_subdirectory(src/windows/wslhost)
488488
add_subdirectory(src/windows/wslrelay)
489+
add_subdirectory(src/windows/wslpluginhost)
489490
add_subdirectory(src/windows/wslinstall)
490491

491492
if (WSL_BUILD_WSL_SETTINGS)

msipackage/CMakeLists.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ set(OUTPUT_PACKAGE ${BIN}/wsl.msi)
1212
set(PACKAGE_WIX_IN ${CMAKE_CURRENT_LIST_DIR}/package.wix.in)
1313
set(PACKAGE_WIX ${BIN}/package.wix)
1414
set(CAB_CACHE ${BIN}/cab)
15-
set(WINDOWS_BINARIES wsl.exe;wslg.exe;wslhost.exe;wslrelay.exe;wslservice.exe;wslserviceproxystub.dll;wslinstall.dll)
15+
set(WINDOWS_BINARIES wsl.exe;wslg.exe;wslhost.exe;wslrelay.exe;wslpluginhost.exe;wslservice.exe;wslserviceproxystub.dll;wslinstall.dll)
1616
if (WSL_BUILD_WSL_SETTINGS)
1717
list(APPEND WINDOWS_BINARIES "wslsettings/wslsettings.dll;wslsettings/wslsettings.exe;libwsl.dll")
1818
endif()
@@ -52,7 +52,7 @@ add_custom_command(
5252

5353
add_custom_target(msipackage DEPENDS ${OUTPUT_PACKAGE})
5454
set_target_properties(msipackage PROPERTIES EXCLUDE_FROM_ALL FALSE SOURCES ${PACKAGE_WIX_IN})
55-
add_dependencies(msipackage wsl wslg wslservice wslhost wslrelay wslserviceproxystub init initramfs wslinstall msixgluepackage)
55+
add_dependencies(msipackage wsl wslg wslservice wslhost wslrelay wslpluginhost wslserviceproxystub init initramfs wslinstall msixgluepackage)
5656

5757
if (WSL_BUILD_WSL_SETTINGS)
5858
add_dependencies(msipackage wslsettings libwsl)

msipackage/package.wix.in

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
<File Id="wslg.exe" Name="wslg.exe" Source="${PACKAGE_INPUT_DIR}/wslg.exe" />
3030
<File Id="wslhost.exe" Name="wslhost.exe" Source="${PACKAGE_INPUT_DIR}/wslhost.exe" />
3131
<File Id="wslrelay.exe" Name="wslrelay.exe" Source="${PACKAGE_INPUT_DIR}/wslrelay.exe" />
32+
<File Id="wslpluginhost.exe" Name="wslpluginhost.exe" Source="${PACKAGE_INPUT_DIR}/wslpluginhost.exe" />
3233
<File Id="wslserviceproxystub.dll" Name="wslserviceproxystub.dll" Source="${PACKAGE_INPUT_DIR}/wslserviceproxystub.dll" />
3334
<File Id="wsldeps.dll" Name="wsldeps.dll" Source="${PACKAGE_INPUT_DIR}/wsldeps.dll" />
3435

@@ -159,6 +160,35 @@
159160
<RegistryValue Value='"[INSTALLDIR]wslhost.exe"' Type="string" />
160161
</RegistryKey>
161162

163+
<!-- WslPluginHost - out-of-process plugin isolation (process spawned by service) -->
164+
<!-- WslPluginHost AppID — SYSTEM-only activation -->
165+
<!-- O:SYG:SYD:(A;;CCDCSW;;;SY) -->
166+
<RegistryKey Root="HKCR" Key="AppID\{7a1d2c3e-4b5f-6a7d-8e9f-0a1b2c3d4e5f}">
167+
<RegistryValue Name="AccessPermission" Value="010004801400000020000000000000002C00000001010000000000051200000001010000000000051200000002001C0001000000000014000B000000010100000000000512000000" Type="binary" />
168+
<RegistryValue Name="LaunchPermission" Value="010004801400000020000000000000002C00000001010000000000051200000001010000000000051200000002001C0001000000000014000B000000010100000000000512000000" Type="binary" />
169+
</RegistryKey>
170+
<RegistryKey Root="HKCR" Key="CLSID\{7a1d2c3e-4b5f-6a7d-8e9f-0a1b2c3d4e5f}">
171+
<RegistryValue Name="AppId" Value="{7a1d2c3e-4b5f-6a7d-8e9f-0a1b2c3d4e5f}" Type="string" />
172+
<RegistryValue Value="WslPluginHost" Type="string" />
173+
</RegistryKey>
174+
<RegistryKey Root="HKCR" Key="CLSID\{7a1d2c3e-4b5f-6a7d-8e9f-0a1b2c3d4e5f}\LocalServer32">
175+
<RegistryValue Value='"[INSTALLDIR]wslpluginhost.exe"' Type="string" />
176+
</RegistryKey>
177+
178+
<!-- WslPluginHost interfaces — use the shared wslserviceproxystub.dll -->
179+
<RegistryKey Root="HKCR" Key="Interface\{A2B3C4D5-E6F7-4890-AB12-CD34EF56A789}">
180+
<RegistryValue Value="IWslPluginHostCallback" Type="string" />
181+
<RegistryKey Key="ProxyStubClsid32">
182+
<RegistryValue Value="{4EA0C6DD-E9FF-48E7-994E-13A31D10DC60}" Type="string" />
183+
</RegistryKey>
184+
</RegistryKey>
185+
<RegistryKey Root="HKCR" Key="Interface\{B3C4D5E6-F7A8-4901-BC23-DE45FA67B890}">
186+
<RegistryValue Value="IWslPluginHost" Type="string" />
187+
<RegistryKey Key="ProxyStubClsid32">
188+
<RegistryValue Value="{4EA0C6DD-E9FF-48E7-994E-13A31D10DC60}" Type="string" />
189+
</RegistryKey>
190+
</RegistryKey>
191+
162192
<!-- wsldevicehost.dll -->
163193
<RegistryKey Root="HKCR" Key="AppID\{17696EAC-9568-4CF5-BB8C-82515AAD6C09}">
164194
<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
@@ -83,6 +83,7 @@ Module Name:
8383
#include <optional>
8484
#include <filesystem>
8585
#include <mutex>
86+
#include <shared_mutex>
8687
#include <chrono>
8788
#include <codecvt>
8889
#include <random>

src/windows/service/exe/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ set(HEADERS
5050
WslCoreVm.h)
5151

5252
add_executable(wslservice ${SOURCES} ${HEADERS})
53-
add_dependencies(wslservice wslserviceidl wslservicemc)
53+
add_dependencies(wslservice wslserviceidl wslservicemc wslpluginhostidl)
5454
add_compile_definitions(__WRL_CLASSIC_COM__)
5555
add_compile_definitions(__WRL_DISABLE_STATIC_INITIALIZE__)
5656
add_compile_definitions(USE_COM_CONTEXT_DEF=1)

src/windows/service/exe/LxssUserSession.cpp

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2604,13 +2604,18 @@ std::shared_ptr<LxssRunningInstance> LxssUserSessionImpl::_CreateInstance(_In_op
26042604
registration.Write(Property::OsVersion, distributionInfo->Version);
26052605
}
26062606

2607-
// This needs to be done before plugins are notifed because they might try to run a command inside the distribution.
2608-
m_runningInstances[registration.Id()] = instance;
2607+
// This needs to be done before plugins are notified because they might try to run a command inside the distribution.
2608+
{
2609+
std::unique_lock callbackLock(m_callbackLock);
2610+
m_runningInstances[registration.Id()] = instance;
2611+
}
26092612

26102613
if (version == LXSS_WSL_VERSION_2)
26112614
{
2612-
auto cleanupOnFailure =
2613-
wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { m_runningInstances.erase(registration.Id()); });
2615+
auto cleanupOnFailure = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() {
2616+
std::unique_lock callbackLock(m_callbackLock);
2617+
m_runningInstances.erase(registration.Id());
2618+
});
26142619
m_pluginManager.OnDistributionStarted(&m_session, instance->DistributionInformation());
26152620
cleanupOnFailure.release();
26162621
}
@@ -3580,14 +3585,18 @@ bool LxssUserSessionImpl::_TerminateInstanceInternal(_In_ LPCGUID DistroGuid, _I
35803585
instance->second->Stop();
35813586

35823587
const auto clientId = instance->second->GetClientId();
3583-
{
3584-
auto lock = m_terminatedInstanceLock.lock_exclusive();
3585-
m_terminatedInstances.push_back(std::move(instance->second));
3586-
}
35873588

35883589
m_lifetimeManager.RemoveCallback(clientKey);
35893590

3590-
m_runningInstances.erase(instance);
3591+
{
3592+
std::unique_lock callbackLock(m_callbackLock);
3593+
{
3594+
auto lock = m_terminatedInstanceLock.lock_exclusive();
3595+
m_terminatedInstances.push_back(std::move(instance->second));
3596+
}
3597+
3598+
m_runningInstances.erase(instance);
3599+
}
35913600

35923601
// If the instance that was terminated was a WSL2 instance,
35933602
// check if the VM is now idle.
@@ -3615,7 +3624,10 @@ void LxssUserSessionImpl::_UpdateInit(_In_ const LXSS_DISTRO_CONFIGURATION& Conf
36153624

36163625
HRESULT LxssUserSessionImpl::MountRootNamespaceFolder(_In_ LPCWSTR HostPath, _In_ LPCWSTR GuestPath, _In_ bool ReadOnly, _In_ LPCWSTR Name)
36173626
{
3618-
std::lock_guard lock(m_instanceLock);
3627+
// Shared lock prevents _VmTerminate from destroying the VM while we use it.
3628+
// Do NOT acquire m_instanceLock — callbacks arrive on a different COM thread
3629+
// from the notification thread that holds m_instanceLock.
3630+
std::shared_lock lock(m_callbackLock);
36193631
RETURN_HR_IF(E_NOT_VALID_STATE, !m_utilityVm);
36203632

36213633
m_utilityVm->MountRootNamespaceFolder(HostPath, GuestPath, ReadOnly, Name);
@@ -3624,7 +3636,9 @@ HRESULT LxssUserSessionImpl::MountRootNamespaceFolder(_In_ LPCWSTR HostPath, _In
36243636

36253637
HRESULT LxssUserSessionImpl::CreateLinuxProcess(_In_opt_ const GUID* Distro, _In_ LPCSTR Path, _In_ LPCSTR* Arguments, _Out_ SOCKET* Socket)
36263638
{
3627-
std::lock_guard lock(m_instanceLock);
3639+
// Shared lock prevents _VmTerminate from destroying the VM or instances
3640+
// while we use them. See MountRootNamespaceFolder for rationale.
3641+
std::shared_lock lock(m_callbackLock);
36283642
RETURN_HR_IF(E_NOT_VALID_STATE, !m_utilityVm);
36293643

36303644
if (Distro == nullptr)
@@ -3871,7 +3885,12 @@ void LxssUserSessionImpl::_VmTerminate()
38713885
m_telemetryThread.join();
38723886
}
38733887

3874-
m_utilityVm.reset();
3888+
// Acquire exclusive callback lock to wait for any in-flight plugin callbacks
3889+
// (MountRootNamespaceFolder, CreateLinuxProcess) to complete before destroying the VM.
3890+
{
3891+
std::unique_lock callbackLock(m_callbackLock);
3892+
m_utilityVm.reset();
3893+
}
38753894
m_vmId.store(GUID_NULL);
38763895

38773896
// 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)