From 1e0c75c3bd8a7b5c25189410f7ecbde643bb3873 Mon Sep 17 00:00:00 2001 From: ergo720 <45463469+ergo720@users.noreply.github.com> Date: Sun, 4 Apr 2021 17:55:02 +0200 Subject: [PATCH] Detect mismatch between cxbxr modules --- projects/cxbxr-ldr/CMakeLists.txt | 2 ++ src/CxbxVersion.h | 9 +++++++++ src/common/win32/EmuShared.cpp | 1 + src/common/win32/EmuShared.h | 20 +++++++++++++++++--- src/core/kernel/init/CxbxKrnl.cpp | 11 +++++++++++ src/core/kernel/init/CxbxKrnl.h | 3 --- src/emulator/cxbxr-emu.cpp | 9 +++++++++ src/loader/cxbxr-ldr.cpp | 9 +++++++++ 8 files changed, 58 insertions(+), 6 deletions(-) diff --git a/projects/cxbxr-ldr/CMakeLists.txt b/projects/cxbxr-ldr/CMakeLists.txt index 4bfc47151..1e629bea8 100644 --- a/projects/cxbxr-ldr/CMakeLists.txt +++ b/projects/cxbxr-ldr/CMakeLists.txt @@ -25,6 +25,8 @@ endif() file (GLOB HEADERS "${CXBXR_ROOT_DIR}/src/common/AddressRanges.h" "${CXBXR_ROOT_DIR}/src/common/ReserveAddressRanges.h" + "${CXBXR_ROOT_DIR}/src/CxbxVersion.h" + "${CXBXR_ROOT_DIR}/src/version.h" ) file (GLOB SOURCES diff --git a/src/CxbxVersion.h b/src/CxbxVersion.h index 17983832b..e4f1e1969 100644 --- a/src/CxbxVersion.h +++ b/src/CxbxVersion.h @@ -1,5 +1,14 @@ #pragma once #include "version.h" +#include + extern const char* CxbxVersionStr; + +// Note: GitVersionMaxLength should be large enough to accomodate the longest git version string we can practically expect to have. This is necessary +// to avoid possible mismatches in the string length which can happen if the user mixes different cxbxr versions +inline constexpr const char *CxbxGitVersion = _GIT_VERSION; +inline constexpr size_t GitVersionLength = std::char_traits::length(CxbxGitVersion); +inline constexpr size_t GitVersionMaxLength = 80; +static_assert(GitVersionLength < GitVersionMaxLength); diff --git a/src/common/win32/EmuShared.cpp b/src/common/win32/EmuShared.cpp index e3ecea7d8..1d8f88367 100644 --- a/src/common/win32/EmuShared.cpp +++ b/src/common/win32/EmuShared.cpp @@ -168,6 +168,7 @@ EmuShared::EmuShared() for (auto& i : m_DeviceType) { i = to_underlying(XBOX_INPUT_DEVICE::DEVICE_INVALID); } + std::strncpy(m_git_version, CxbxGitVersion, GitVersionLength); } // ****************************************************************** diff --git a/src/common/win32/EmuShared.h b/src/common/win32/EmuShared.h index 4fcd35b70..93da9711b 100644 --- a/src/common/win32/EmuShared.h +++ b/src/common/win32/EmuShared.h @@ -30,8 +30,8 @@ #include "Mutex.h" #include "common\IPCHybrid.hpp" #include "common\input\Button.h" +#include "CxbxVersion.h" #include "core/common/imgui/settings.h" - #include extern HMODULE hActiveModule; // Equals EXE Module handle in (GUI) Cxbx.exe / cxbxr.exe, equals DLL Module handle in cxbxr-emu.dll @@ -66,6 +66,11 @@ class EmuShared : public Mutex // ****************************************************************** unsigned int m_size; + // ****************************************************************** + // * Git version string of cxbx.exe + // ****************************************************************** + char m_git_version[GitVersionMaxLength]; + // ****************************************************************** // * Each process needs to call this to initialize shared memory // ****************************************************************** @@ -286,6 +291,16 @@ class EmuShared : public Mutex void GetOverlaySettings(overlay_settings *value) { Lock(); *value = m_imgui_overlay_settings; Unlock(); } void SetOverlaySettings(const overlay_settings* value) { Lock(); m_imgui_overlay_settings = *value; Unlock(); } + // ****************************************************************** + // * Git version Accessor (only the get method is provided because it should not be changed + // ****************************************************************** + void GetGitVersion(char *value) + { + Lock(); + std::strncpy(value, m_git_version, GitVersionLength + 1); + Unlock(); + } + // ****************************************************************** // * Reset specific variables to default for kernel mode. // ****************************************************************** @@ -329,9 +344,8 @@ class EmuShared : public Mutex bool m_bEmulating_status; #ifndef CXBX_LOADER // Temporary usage for cxbx.exe's emu unsigned int m_PreviousMmLayout; - int m_Reserved7[3]; #else - int m_Reserved7[4]; + unsigned int m_Reserved; #endif bool m_bFirstLaunch; bool m_bClipCursor; diff --git a/src/core/kernel/init/CxbxKrnl.cpp b/src/core/kernel/init/CxbxKrnl.cpp index b9e96f990..8c245c436 100644 --- a/src/core/kernel/init/CxbxKrnl.cpp +++ b/src/core/kernel/init/CxbxKrnl.cpp @@ -685,6 +685,17 @@ bool HandleFirstLaunch() void CxbxKrnlEmulate(unsigned int reserved_systems, blocks_reserved_t blocks_reserved) { +#ifdef CXBXR_EMU + // First of all, check if the emulation dll version matches the gui version and abort otherwise + char GitVersionGui[GitVersionMaxLength]; + g_EmuShared->GetGitVersion(GitVersionGui); + if (std::strncmp(GitVersionGui, CxbxGitVersion, GitVersionLength) != 0) { + PopupError(nullptr, "Mismatch detected between cxbx.exe and cxbxr-emu.dll, aborting."); + CxbxKrnlShutDown(); + return; + } +#endif + std::string tempStr; // NOTE: This is designated for standalone kernel mode launch without GUI diff --git a/src/core/kernel/init/CxbxKrnl.h b/src/core/kernel/init/CxbxKrnl.h index 293b8728e..f4df488e6 100644 --- a/src/core/kernel/init/CxbxKrnl.h +++ b/src/core/kernel/init/CxbxKrnl.h @@ -136,9 +136,6 @@ extern "C" { extern Xbe::Certificate *g_pCertificate; -/*! validate version string match */ -bool CxbxKrnlVerifyVersion(const char *szVersion); - extern bool g_bIsDebugKernel; bool CreateSettings(); diff --git a/src/emulator/cxbxr-emu.cpp b/src/emulator/cxbxr-emu.cpp index 44ba5f417..1a0d5d174 100644 --- a/src/emulator/cxbxr-emu.cpp +++ b/src/emulator/cxbxr-emu.cpp @@ -166,6 +166,15 @@ DWORD WINAPI Emulate(unsigned int reserved_systems, blocks_reserved_t blocks_res return EXIT_FAILURE; } + // Check if the loader version matches the gui version and abort otherwise + char GitVersionGui[GitVersionMaxLength]; + g_EmuShared->GetGitVersion(GitVersionGui); + if (std::strncmp(GitVersionGui, reinterpret_cast(PHYSICAL_MAP1_BASE + 0x1000), GitVersionLength) != 0) { + PopupError(nullptr, "Mismatch detected between cxbx.exe and cxbxr-ldr.exe, aborting."); + EmuShared::Cleanup(); + return EXIT_FAILURE; + } + if (!HandleFirstLaunch()) { PopupError(nullptr, "First launch failed!"); EmuShared::Cleanup(); diff --git a/src/loader/cxbxr-ldr.cpp b/src/loader/cxbxr-ldr.cpp index a967ec2f1..a24463ad3 100644 --- a/src/loader/cxbxr-ldr.cpp +++ b/src/loader/cxbxr-ldr.cpp @@ -28,7 +28,9 @@ #define WIN32_LEAN_AND_MEAN // Exclude rarely-used stuff from Windows headers #include // For LPTSTR, FormatMessage, GetSystemInfo, etc +#include "strsafe.h" // For StringCchCopy +#include "..\CxbxVersion.h" #include "..\Common\AddressRanges.h" #include "..\Common\ReserveAddressRanges.h" @@ -193,6 +195,13 @@ DWORD CALLBACK rawMain() return ERROR_RESOURCE_NOT_FOUND; } + // We cannot just pass the gui version of the loader via the Emulate function. This, because if the user mixes a version which does the check (and thus expects 3 arguments) + // with an old version which doesn't do the check (and thus only has 2 arguments), the behavior will be undefined since the new version will attempt to use a + // non-existent argument. We instead pass the version string in the contiguous memory, which must have been successfully reserved by now or else the loader would + // have already aborted execution. This memory is backed by the paging file, and thus its contents will always be initialized to zero. Thus, in the above scenerio + // the check will fail because a version string cannot be zero. + StringCchCopy(reinterpret_cast(PHYSICAL_MAP1_BASE + 0x1000), GitVersionLength + 1, CxbxGitVersion); + // Find the main emulation function in our DLL typedef void (WINAPI *Emulate_t)(unsigned int, blocks_reserved_t); Emulate_t pfnEmulate = (Emulate_t)GetProcAddress(hEmulationDLL, "Emulate");