Merge pull request #2182 from ergo720/git_version_check

Detect mismatches between the cxbxr executables

projects/cxbxr-ldr/CMakeLists.txt

@@ -25,11 +25,14 @@ 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
  "${CXBXR_ROOT_DIR}/src/common/AddressRanges.cpp"
  "${CXBXR_ROOT_DIR}/src/common/ReserveAddressRanges.cpp"
+ "${CXBXR_ROOT_DIR}/src/CxbxVersion.cpp"
  "${CXBXR_ROOT_DIR}/src/loader/cxbxr-ldr.cpp"
 )
 
@@ -68,3 +71,5 @@ target_link_libraries(cxbxr-ldr
 install(TARGETS ${PROJECT_NAME}
   RUNTIME DESTINATION bin
 )
+
+add_dependencies(cxbxr-ldr misc-batch)

src/CxbxVersion.cpp

@@ -2,6 +2,7 @@
 // PVS-Studio Static Code Analyzer for C, C++ and C#: http://www.viva64.com
 #include "Version.h"
 #include "CxbxVersion.h"
+#include <string>
 
 /*! version string dependent on trace flag */
 #ifndef _DEBUG_TRACE
@@ -9,3 +10,15 @@ const char* CxbxVersionStr = _GIT_VERSION " (" __DATE__  ")";
 #else
 const char* CxbxVersionStr = _GIT_VERSION "-Trace (" __DATE__  ")";
 #endif
+
+static constexpr const char *GitVersionStr = _GIT_VERSION;
+static constexpr size_t GitVersionLength = std::char_traits<char>::length(GitVersionStr);
+static_assert(GitVersionLength < GitVersionMaxLength);
+
+const char *const GetGitVersionStr() {
+    return GitVersionStr;
+}
+
+const size_t GetGitVersionLength() {
+    return GitVersionLength;
+}

src/CxbxVersion.h

@@ -1,5 +1,9 @@
 #pragma once
 
-#include "version.h"
-
 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 size_t GitVersionMaxLength = 80;
+const char *const GetGitVersionStr();
+const size_t GetGitVersionLength();

src/common/win32/EmuShared.cpp

@@ -153,12 +153,6 @@ EmuShared::EmuShared()
 	m_bFirstLaunch = false;
 	m_bClipCursor = false;
 
-	// Reserve space (default to 0)
-	m_bReserved4 = false;
-	m_Reserved5 = 0;
-	m_Reserved6 = 0.0f;
-	std::memset(m_Reserved7, 0, sizeof(m_Reserved7));
-	std::memset(m_Reserved99, 0, sizeof(m_Reserved99));
 	std::memset(m_DeviceControlNames, '\0', sizeof(m_DeviceControlNames));
 	std::memset(m_DeviceName, '\0', sizeof(m_DeviceName));
 	m_imgui_general.ini_size = IMGUI_INI_SIZE_MAX;
@@ -168,6 +162,7 @@ EmuShared::EmuShared()
 	for (auto& i : m_DeviceType) {
 		i = to_underlying(XBOX_INPUT_DEVICE::DEVICE_INVALID);
 	}
+    std::strncpy(m_git_version, GetGitVersionStr(), GetGitVersionLength());
 }
 
 // ******************************************************************

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 <memory.h>
 
 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 the executable that first launched
+		// ******************************************************************
+		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, GetGitVersionLength() + 1);
+			Unlock();
+		}
+
 		// ******************************************************************
 		// * Reset specific variables to default for kernel mode.
 		// ******************************************************************
@@ -320,8 +335,6 @@ class EmuShared : public Mutex
 		// * Shared configuration
 		// ******************************************************************
 		int          m_BootFlags_status;
-		unsigned int m_Reserved5;
-		float        m_Reserved6;
 		float        m_FPS_status; // NOTE: If move into ipc_send_gui_update will spam GUI's message system (one message per frame)
 		bool         m_Krnl_Log_enabled; // Is require in order to preserve previous set for support multi-xbe.
 		bool         m_bDebugging;
@@ -329,19 +342,15 @@ 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;
-		bool         m_bReserved3;
-		bool         m_bReserved4;
 		unsigned int m_dwKrnlProcID; // Only used for kernel mode level.
 		int          m_DeviceType[4];
 		char         m_DeviceControlNames[4][HIGHEST_NUM_BUTTONS][HOST_BUTTON_NAME_LENGTH];
 		char         m_DeviceName[4][50];
-		int          m_Reserved99[28]; // Reserve space
 
 		// Settings class in memory should not be tampered by third-party.
 		// Third-party program should only be allow to edit settings.ini file.

src/core/kernel/init/CxbxKrnl.cpp

@@ -685,6 +685,16 @@ bool HandleFirstLaunch()
 
 void CxbxKrnlEmulate(unsigned int reserved_systems, blocks_reserved_t blocks_reserved)
 {
+	// First of all, check if the EmuShared version matches the emu version and abort otherwise
+	char GitVersionEmuShared[GitVersionMaxLength];
+	g_EmuShared->GetGitVersion(GitVersionEmuShared);
+	if (std::strncmp(GitVersionEmuShared, GetGitVersionStr(), GetGitVersionLength()) != 0) {
+		PopupError(nullptr, "Mismatch detected between EmuShared and cxbx.exe/cxbxr-emu.dll, aborting."
+			"\n\nPlease extract all contents from zip file and do not mix with older/newer builds.");
+		CxbxKrnlShutDown();
+		return;
+	}
+
 	std::string tempStr;
 
 	// NOTE: This is designated for standalone kernel mode launch without GUI

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();

src/emulator/cxbxr-emu.cpp

@@ -166,6 +166,13 @@ DWORD WINAPI Emulate(unsigned int reserved_systems, blocks_reserved_t blocks_res
 		return EXIT_FAILURE;
 	}
 
+	// Check if the loader version matches the emu version and abort otherwise
+	if (std::strncmp(GetGitVersionStr(), reinterpret_cast<char *>(PHYSICAL_MAP1_BASE + 0x1000), GetGitVersionLength()) != 0) {
+		PopupError(nullptr, "Mismatch detected between cxbxr-ldr.exe and cxbxr-emu.dll, aborting.");
+		EmuShared::Cleanup();
+		return EXIT_FAILURE;
+	}
+
 	if (!HandleFirstLaunch()) {
 		PopupError(nullptr, "First launch failed!");
 		EmuShared::Cleanup();

src/loader/cxbxr-ldr.cpp

@@ -29,6 +29,7 @@
 #define WIN32_LEAN_AND_MEAN             // Exclude rarely-used stuff from Windows headers
 #include <Windows.h> // For LPTSTR, FormatMessage, GetSystemInfo, etc
 
+#include "..\CxbxVersion.h"
 #include "..\Common\AddressRanges.h"
 #include "..\Common\ReserveAddressRanges.h"
 
@@ -193,6 +194,17 @@ DWORD CALLBACK rawMain()
 		return ERROR_RESOURCE_NOT_FOUND;
 	}
 
+	// We cannot just pass the git 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.
+	// NOTE1: the loader doesn't link against the CRT, which means we cannot just use strncpy here, and thus we use a for loop instead
+	// NOTE2: we choose 0x80001000 as address because the first page is used by d3d to initialize the push buffer of the nv2a, so we avoid to write a string to it
+	for (unsigned i = 0; i < GetGitVersionLength(); ++i) {
+		*(reinterpret_cast<char *>(PHYSICAL_MAP1_BASE + 0x1000) + i) = GetGitVersionStr()[i];
+	}
+
 	// 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");