diff --git a/msipackage/package.wix.in b/msipackage/package.wix.in index 2584248b6..ba3957190 100644 --- a/msipackage/package.wix.in +++ b/msipackage/package.wix.in @@ -314,6 +314,14 @@ + + + + + + + + diff --git a/src/windows/WslcSDK/CMakeLists.txt b/src/windows/WslcSDK/CMakeLists.txt index d9f0b7e77..8c026430f 100644 --- a/src/windows/WslcSDK/CMakeLists.txt +++ b/src/windows/WslcSDK/CMakeLists.txt @@ -2,6 +2,7 @@ set(SOURCES IOCallback.cpp ProgressCallback.cpp TerminationCallback.cpp + CrashDumpCallback.cpp wslcsdk.cpp WslcsdkPrivate.cpp ) @@ -10,6 +11,7 @@ set(HEADERS IOCallback.h ProgressCallback.h TerminationCallback.h + CrashDumpCallback.h wslcsdk.h WslcsdkPrivate.h ) diff --git a/src/windows/WslcSDK/CrashDumpCallback.cpp b/src/windows/WslcSDK/CrashDumpCallback.cpp new file mode 100644 index 000000000..406edb877 --- /dev/null +++ b/src/windows/WslcSDK/CrashDumpCallback.cpp @@ -0,0 +1,52 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + CrashDumpCallback.cpp + +Abstract: + + Implementation of a type that implements ICrashDumpCallback. + +--*/ +#include "precomp.h" +#include "CrashDumpCallback.h" + +CrashDumpCallback::CrashDumpCallback(WslcSessionCrashDumpCallback callback, PVOID context) : + m_callback(callback), m_context(context) +{ +} + +HRESULT STDMETHODCALLTYPE CrashDumpCallback::OnCrashDump( + _In_ LPCWSTR DumpPath, _In_opt_ LPCSTR ProcessName, _In_ ULONGLONG Pid, _In_ ULONG Signal, _In_ ULONGLONG Timestamp) +try +{ + if (m_callback) + { + WslcSessionCrashDumpInfo info{}; + info.dumpPath = DumpPath; + info.processName = ProcessName ? ProcessName : ""; + info.pid = Pid; + info.signal = Signal; + info.timestamp = Timestamp; + + m_callback(&info, m_context); + } + + return S_OK; +} +CATCH_RETURN(); + +winrt::com_ptr CrashDumpCallback::CreateIf(const WslcSessionOptionsInternal* options) +{ + if (options->crashDumpCallback) + { + return winrt::make_self(options->crashDumpCallback, options->crashDumpCallbackContext); + } + else + { + return nullptr; + } +} diff --git a/src/windows/WslcSDK/CrashDumpCallback.h b/src/windows/WslcSDK/CrashDumpCallback.h new file mode 100644 index 000000000..1cfd24c9a --- /dev/null +++ b/src/windows/WslcSDK/CrashDumpCallback.h @@ -0,0 +1,34 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + CrashDumpCallback.h + +Abstract: + + Header for a type that implements ICrashDumpCallback. Bridges the COM + ICrashDumpCallback interface back to the C-style SDK callback registered + via WslcSetSessionSettingsCrashDumpCallback. + +--*/ +#pragma once +#include "wslc.h" +#include "wslcsdkprivate.h" +#include + +struct CrashDumpCallback : public winrt::implements +{ + CrashDumpCallback(WslcSessionCrashDumpCallback callback, PVOID context); + + // ICrashDumpCallback + HRESULT STDMETHODCALLTYPE OnCrashDump(_In_ LPCWSTR DumpPath, _In_opt_ LPCSTR ProcessName, _In_ ULONGLONG Pid, _In_ ULONG Signal, _In_ ULONGLONG Timestamp) override; + + // Creates a CrashDumpCallback if the options provides a callback. + static winrt::com_ptr CreateIf(const WslcSessionOptionsInternal* options); + +private: + WslcSessionCrashDumpCallback m_callback = nullptr; + PVOID m_context = nullptr; +}; diff --git a/src/windows/WslcSDK/WslcsdkPrivate.h b/src/windows/WslcSDK/WslcsdkPrivate.h index 36a456893..bde141e5a 100644 --- a/src/windows/WslcSDK/WslcsdkPrivate.h +++ b/src/windows/WslcSDK/WslcsdkPrivate.h @@ -35,6 +35,9 @@ typedef struct WslcSessionOptionsInternal WslcSessionFeatureFlags featureFlags; WslcSessionTerminationCallback terminationCallback; PVOID terminationCallbackContext; + + WslcSessionCrashDumpCallback crashDumpCallback; + PVOID crashDumpCallbackContext; } WslcSessionOptionsInternal; static_assert(sizeof(WslcSessionOptionsInternal) == WSLC_SESSION_OPTIONS_SIZE, "WSLC_SESSION_OPTIONS_INTERNAL size mismatch"); @@ -108,6 +111,7 @@ struct WslcSessionImpl { wil::com_ptr session; wil::com_ptr terminationCallback; + wil::com_ptr crashDumpCallback; }; WslcSessionImpl* GetInternalType(WslcSession handle); diff --git a/src/windows/WslcSDK/wslcsdk.cpp b/src/windows/WslcSDK/wslcsdk.cpp index 69366079a..00d91520e 100644 --- a/src/windows/WslcSDK/wslcsdk.cpp +++ b/src/windows/WslcSDK/wslcsdk.cpp @@ -18,6 +18,7 @@ Module Name: #include "Defaults.h" #include "ProgressCallback.h" #include "TerminationCallback.h" +#include "CrashDumpCallback.h" #include "Localization.h" #include "WslInstall.h" #include "wslutil.h" @@ -440,6 +441,12 @@ try result->terminationCallback.attach(terminationCallback.as().detach()); runtimeSettings.TerminationCallback = terminationCallback.get(); } + auto crashDumpCallback = CrashDumpCallback::CreateIf(internalType); + if (crashDumpCallback) + { + result->crashDumpCallback.attach(crashDumpCallback.as().detach()); + runtimeSettings.CrashDumpCallback = crashDumpCallback.get(); + } runtimeSettings.FeatureFlags = ConvertFlags(internalType->featureFlags); WI_SetFlag(runtimeSettings.FeatureFlags, WslcFeatureFlagsVirtioFs); WI_SetFlag(runtimeSettings.FeatureFlags, WslcFeatureFlagsDnsTunneling); @@ -599,6 +606,20 @@ try } CATCH_RETURN(); +STDAPI WslcSetSessionSettingsCrashDumpCallback( + _In_ WslcSessionSettings* sessionSettings, _In_opt_ WslcSessionCrashDumpCallback crashDumpCallback, _In_opt_ PVOID crashDumpContext) +try +{ + auto internalType = CheckAndGetInternalType(sessionSettings); + RETURN_HR_IF(E_INVALIDARG, crashDumpCallback == nullptr && crashDumpContext != nullptr); + + internalType->crashDumpCallback = crashDumpCallback; + internalType->crashDumpCallbackContext = crashDumpContext; + + return S_OK; +} +CATCH_RETURN(); + STDAPI WslcReleaseSession(_In_ WslcSession session) try { @@ -608,6 +629,7 @@ try // the termination callback ends up being invoked by session destruction. internalType->session.reset(); internalType->terminationCallback.reset(); + internalType->crashDumpCallback.reset(); return S_OK; } diff --git a/src/windows/WslcSDK/wslcsdk.def b/src/windows/WslcSDK/wslcsdk.def index 23a3a1c7f..a4d13353b 100644 --- a/src/windows/WslcSDK/wslcsdk.def +++ b/src/windows/WslcSDK/wslcsdk.def @@ -21,6 +21,7 @@ WslcSetSessionSettingsCpuCount WslcSetSessionSettingsMemory WslcSetSessionSettingsTimeout WslcSetSessionSettingsVhd +WslcSetSessionSettingsCrashDumpCallback WslcTerminateSession WslcSessionAuthenticate diff --git a/src/windows/WslcSDK/wslcsdk.h b/src/windows/WslcSDK/wslcsdk.h index 2abbbc8b4..7bc4f4a68 100644 --- a/src/windows/WslcSDK/wslcsdk.h +++ b/src/windows/WslcSDK/wslcsdk.h @@ -42,7 +42,7 @@ EXTERN_C_START #define WSLC_E_REGISTRY_BLOCKED_BY_POLICY MAKE_HRESULT(SEVERITY_ERROR, FACILITY_ITF, WSLC_E_BASE + 13) /* 0x8004060D */ // Session values -#define WSLC_SESSION_OPTIONS_SIZE 88 +#define WSLC_SESSION_OPTIONS_SIZE 104 #define WSLC_SESSION_OPTIONS_ALIGNMENT 8 typedef struct WslcSessionSettings @@ -125,6 +125,17 @@ typedef enum WslcSessionTerminationReason typedef __callback void(CALLBACK* WslcSessionTerminationCallback)(_In_ WslcSessionTerminationReason reason, _In_opt_ PVOID context); +typedef struct WslcSessionCrashDumpInfo +{ + _Field_z_ PCWSTR dumpPath; + _Field_z_ PCSTR processName; + uint64_t pid; + uint32_t signal; + uint64_t timestamp; +} WslcSessionCrashDumpInfo; + +typedef __callback void(CALLBACK* WslcSessionCrashDumpCallback)(_In_ const WslcSessionCrashDumpInfo* info, _In_opt_ PVOID context); + STDAPI WslcInitSessionSettings(_In_ PCWSTR name, _In_ PCWSTR storagePath, _Out_ WslcSessionSettings* sessionSettings); STDAPI WslcCreateSession(_In_ WslcSessionSettings* sessionSettings, _Out_ WslcSession* session, _Outptr_opt_result_z_ PWSTR* errorMessage); @@ -142,6 +153,9 @@ STDAPI WslcSetSessionSettingsFeatureFlags(_In_ WslcSessionSettings* sessionSetti STDAPI WslcSetSessionSettingsTerminationCallback( _In_ WslcSessionSettings* sessionSettings, _In_opt_ WslcSessionTerminationCallback terminationCallback, _In_opt_ PVOID terminationContext); +STDAPI WslcSetSessionSettingsCrashDumpCallback( + _In_ WslcSessionSettings* sessionSettings, _In_opt_ WslcSessionCrashDumpCallback crashDumpCallback, _In_opt_ PVOID crashDumpContext); + STDAPI WslcTerminateSession(_In_ WslcSession session); STDAPI WslcReleaseSession(_In_ WslcSession session); diff --git a/src/windows/service/exe/WSLCSessionManager.cpp b/src/windows/service/exe/WSLCSessionManager.cpp index 700d570d7..6331b26ca 100644 --- a/src/windows/service/exe/WSLCSessionManager.cpp +++ b/src/windows/service/exe/WSLCSessionManager.cpp @@ -427,6 +427,7 @@ WSLCSessionInitSettings WSLCSessionManagerImpl::CreateSessionSettings( sessionSettings.RootVhdTypeOverride = Settings->RootVhdTypeOverride; sessionSettings.StorageFlags = Settings->StorageFlags; sessionSettings.SwapSizeMb = Settings->MemoryMb; + sessionSettings.CrashDumpCallback = Settings->CrashDumpCallback; return sessionSettings; } diff --git a/src/windows/service/inc/wslc.idl b/src/windows/service/inc/wslc.idl index a1a89dc9f..4cebe3b1f 100644 --- a/src/windows/service/inc/wslc.idl +++ b/src/windows/service/inc/wslc.idl @@ -107,6 +107,21 @@ interface ITerminationCallback : IUnknown HRESULT OnTermination(WSLCVirtualMachineTerminationReason Reason, LPCWSTR Details); }; +[ + uuid(8C5A7B14-9D26-4FAE-AB31-7E5BC23F4801), + pointer_default(unique), + object +] +interface ICrashDumpCallback : IUnknown +{ + HRESULT OnCrashDump( + [in, string] LPCWSTR DumpPath, + [in, unique, string] LPCSTR ProcessName, + [in] ULONGLONG Pid, + [in] ULONG Signal, + [in] ULONGLONG Timestamp); +}; + [ uuid(5038842F-53DB-4F30-A6D0-A41B02C94AC1), pointer_default(unique), @@ -526,6 +541,7 @@ typedef struct _WSLCSessionSettings { // Below options are used for debugging purposes only. [unique] LPCWSTR RootVhdOverride; [unique] LPCSTR RootVhdTypeOverride; + [unique] ICrashDumpCallback* CrashDumpCallback; } WSLCSessionSettings; typedef enum _WSLCLogsFlags @@ -716,6 +732,7 @@ typedef struct _WSLCSessionInitSettings WSLCNetworkingMode NetworkingMode; WSLCFeatureFlags FeatureFlags; [unique] LPCSTR RootVhdTypeOverride; + [unique] ICrashDumpCallback* CrashDumpCallback; } WSLCSessionInitSettings; [ diff --git a/src/windows/wslcsession/WSLCVirtualMachine.cpp b/src/windows/wslcsession/WSLCVirtualMachine.cpp index ba7748de0..d0f5f378c 100644 --- a/src/windows/wslcsession/WSLCVirtualMachine.cpp +++ b/src/windows/wslcsession/WSLCVirtualMachine.cpp @@ -254,6 +254,7 @@ WSLCVirtualMachine::WSLCVirtualMachine(_In_ IWSLCVirtualMachine* Vm, _In_ const m_networkingMode(Settings->NetworkingMode), m_bootTimeoutMs(Settings->BootTimeoutMs), m_rootVhdType(Settings->RootVhdTypeOverride ? Settings->RootVhdTypeOverride : "ext4"), + m_crashDumpCallback(Settings->CrashDumpCallback), m_sessionTerminatingEvent(SessionTerminatingEvent) { // N.B. The constructor should not run any operation that could throw, so the destructor runs even if the VM fails to boot. @@ -1247,6 +1248,8 @@ void WSLCVirtualMachine::CollectCrashDumps(wil::unique_socket&& listenSocket) // No impersonation needed - the session process already runs as the user. wslutil::SetThreadDescription(L"CrashDumpCollection"); + const auto coInit = wil::CoInitializeEx(COINIT_MULTITHREADED); + const auto crashDumpFolder = filesystem::GetTempFolderPath(GetCurrentProcessToken()) / L"wslc-crashes"; while (!m_vmTerminatingEvent.is_signaled()) @@ -1273,10 +1276,14 @@ void WSLCVirtualMachine::CollectCrashDumps(wil::unique_socket&& listenSocket) const auto bufferSize = responseSpan.size_bytes() - offsetof(LX_PROCESS_CRASH, Buffer); const std::string process(message.Buffer, strnlen(message.Buffer, bufferSize)); + const auto crashPid = message.Pid; + const auto crashSignal = message.Signal; + const auto crashTimestamp = message.Timestamp; + constexpr auto dumpExtension = ".dmp"; constexpr auto dumpPrefix = "wsl-crash"; - auto filename = std::format("{}-{}-{}-{}-{}{}", dumpPrefix, message.Timestamp, message.Pid, process, message.Signal, dumpExtension); + auto filename = std::format("{}-{}-{}-{}-{}{}", dumpPrefix, crashTimestamp, crashPid, process, crashSignal, dumpExtension); std::replace_if( filename.begin(), @@ -1289,8 +1296,8 @@ void WSLCVirtualMachine::CollectCrashDumps(wil::unique_socket&& listenSocket) WSL_LOG( "WSLCLinuxCrash", TraceLoggingValue(fullPath.c_str(), "FullPath"), - TraceLoggingValue(message.Pid, "Pid"), - TraceLoggingValue(message.Signal, "Signal"), + TraceLoggingValue(crashPid, "Pid"), + TraceLoggingValue(crashSignal, "Signal"), TraceLoggingValue(process.c_str(), "process")); filesystem::EnsureDirectory(crashDumpFolder.c_str()); @@ -1314,6 +1321,15 @@ void WSLCVirtualMachine::CollectCrashDumps(wil::unique_socket&& listenSocket) transaction.SendResultMessage(0); relay::InterruptableRelay(reinterpret_cast(channel.Socket()), file.get(), nullptr); + + file.reset(); + + // Invoke the crash dump callback (if any) now that the dump file has been fully + // written. Failures in the callback are logged but do not interrupt crash collection. + if (m_crashDumpCallback) + { + LOG_IF_FAILED(m_crashDumpCallback->OnCrashDump(fullPath.c_str(), process.c_str(), crashPid, crashSignal, crashTimestamp)); + } } CATCH_LOG() } diff --git a/src/windows/wslcsession/WSLCVirtualMachine.h b/src/windows/wslcsession/WSLCVirtualMachine.h index a38588a74..204d940f5 100644 --- a/src/windows/wslcsession/WSLCVirtualMachine.h +++ b/src/windows/wslcsession/WSLCVirtualMachine.h @@ -219,6 +219,9 @@ class WSLCVirtualMachine std::string m_rootVhdType; + // Optional callback invoked after a crash dump is successfully written. + wil::com_ptr m_crashDumpCallback; + std::thread m_processExitThread; std::thread m_crashDumpThread; diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index 0650cc26f..91ab66b4d 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -2753,6 +2753,71 @@ class WSLCTests VERIFY_ARE_NOT_EQUAL(details, L""); } + WSLC_TEST_METHOD(CrashDumpCallback) + { + struct Invocation + { + std::wstring DumpPath; + std::string ProcessName; + ULONGLONG Pid; + ULONG Signal; + ULONGLONG Timestamp; + }; + + class DECLSPEC_UUID("8C5A7B14-9D26-4FAE-AB31-7E5BC23F4802") CallbackInstance + : public Microsoft::WRL::RuntimeClass, ICrashDumpCallback, IFastRundown> + { + public: + CallbackInstance(std::promise& promise, wil::unique_event& release) : + m_promise(promise), m_release(release) + { + } + + HRESULT OnCrashDump(LPCWSTR DumpPath, LPCSTR ProcessName, ULONGLONG Pid, ULONG Signal, ULONGLONG Timestamp) override + { + m_promise.set_value(Invocation{ + DumpPath ? std::wstring{DumpPath} : std::wstring{}, ProcessName ? std::string{ProcessName} : std::string{}, Pid, Signal, Timestamp}); + + // Block until the test has finished probing, so anything the test verifies is observed mid-callback. + m_release.wait(); + return S_OK; + } + + private: + std::promise& m_promise; + wil::unique_event& m_release; + }; + + std::promise promise; + wil::unique_event release{wil::EventOptions::ManualReset}; + auto callback = Microsoft::WRL::Make(promise, release); + + WSLCSessionSettings sessionSettings = GetDefaultSessionSettings(L"crash-dump-callback-test"); + sessionSettings.CrashDumpCallback = callback.Get(); + auto session = CreateSession(sessionSettings); + + // Trigger a Linux process crash. The shell exits with 128 + SIGSEGV. + ExpectCommandResult(session.get(), {"/bin/sh", "-c", "kill -SEGV $$"}, 128 + WSLCSignalSIGSEGV); + + auto future = promise.get_future(); + VERIFY_ARE_EQUAL(future.wait_for(std::chrono::seconds(60)), std::future_status::ready); + + // Make sure the callback is unblocked even if a verification below throws. + auto releaseCallback = wil::scope_exit([&]() { release.SetEvent(); }); + auto invocation = future.get(); + VERIFY_IS_FALSE(invocation.DumpPath.empty()); + VERIFY_IS_TRUE(invocation.ProcessName.find("sh") != std::string::npos); + VERIFY_ARE_EQUAL(invocation.Signal, static_cast(WSLCSignalSIGSEGV)); + VERIFY_IS_GREATER_THAN(invocation.Pid, 0ull); + VERIFY_IS_GREATER_THAN(invocation.Timestamp, 0ull); + + // The dump file should be readable and non-empty. + wil::unique_hfile dumpFile{CreateFileW( + invocation.DumpPath.c_str(), GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr)}; + VERIFY_IS_TRUE(dumpFile.is_valid()); + VERIFY_IS_GREATER_THAN(std::filesystem::file_size(invocation.DumpPath), 0ull); + } + WSLC_TEST_METHOD(BuildImageStuckCallbackCancellation) { SKIP_TEST_SERVER(); diff --git a/test/windows/WslcSdkTests.cpp b/test/windows/WslcSdkTests.cpp index e676b4128..13b450175 100644 --- a/test/windows/WslcSdkTests.cpp +++ b/test/windows/WslcSdkTests.cpp @@ -17,6 +17,7 @@ Module Name: #include "wslcsdk.h" #include "WslcsdkPrivate.h" #include "WSLCContainerLauncher.h" +#include "WSLCProcessLauncher.h" #include "wslc_schema.h" #include @@ -319,6 +320,59 @@ class WslcSdkTests VERIFY_ARE_EQUAL(future.get(), WSLC_SESSION_TERMINATION_REASON_SHUTDOWN); } + WSLC_TEST_METHOD(CrashDumpCallback) + { + struct Invocation + { + std::wstring DumpPath; + std::string ProcessName; + uint64_t Pid; + uint32_t Signal; + uint64_t Timestamp; + }; + + std::promise promise; + + auto callback = [](const WslcSessionCrashDumpInfo* info, PVOID context) { + auto* p = static_cast*>(context); + p->set_value(Invocation{ + info->dumpPath ? std::wstring{info->dumpPath} : std::wstring{}, + info->processName ? std::string{info->processName} : std::string{}, + info->pid, + info->signal, + info->timestamp}); + }; + + std::filesystem::path extraStorage = m_storagePath / "wslc-crashcb-storage"; + + WslcSessionSettings sessionSettings; + VERIFY_SUCCEEDED(WslcInitSessionSettings(L"wslc-crashcb-test", extraStorage.c_str(), &sessionSettings)); + VERIFY_SUCCEEDED(WslcSetSessionSettingsTimeout(&sessionSettings, 30 * 1000)); + VERIFY_SUCCEEDED(WslcSetSessionSettingsCrashDumpCallback(&sessionSettings, callback, &promise)); + + UniqueSession session; + VERIFY_SUCCEEDED(WslcCreateSession(&sessionSettings, &session, nullptr)); + + auto& comSession = *reinterpret_cast(session.get())->session; + + wsl::windows::common::WSLCProcessLauncher launcher{"/bin/sh", {"/bin/sh", "-c", "kill -SEGV $$"}}; + auto process = launcher.Launch(comSession); + auto result = process.WaitAndCaptureOutput(); + VERIFY_ARE_EQUAL(result.Code, 128 + WSLCSignalSIGSEGV); + + auto future = promise.get_future(); + VERIFY_ARE_EQUAL(future.wait_for(std::chrono::seconds(60)), std::future_status::ready); + + auto invocation = future.get(); + VERIFY_IS_FALSE(invocation.DumpPath.empty()); + VERIFY_IS_TRUE(std::filesystem::exists(invocation.DumpPath)); + VERIFY_IS_GREATER_THAN(std::filesystem::file_size(invocation.DumpPath), 0ull); + VERIFY_IS_TRUE(invocation.ProcessName.find("sh") != std::string::npos); + VERIFY_ARE_EQUAL(invocation.Signal, static_cast(WSLCSignalSIGSEGV)); + VERIFY_IS_GREATER_THAN(invocation.Pid, 0ull); + VERIFY_IS_GREATER_THAN(invocation.Timestamp, 0ull); + } + // ----------------------------------------------------------------------- // Image tests // -----------------------------------------------------------------------