iidx, sdvx: bi2x hook fix (attempt 2) (#499)

## Link to GitHub Issue, if one exists
see my previous attempt #496, which has been reverted by #500

## Description of change
Two issues:

1. the original implementation of bi2x_hook for iidx misunderstood what
`aioNMgrIob2_Create` does. In the original I/O code it returns a pointer
to a C++ class instance, which would also have a vptr (pointer to a
vtable) in the beginning. The original author of this code of the hook
in spice thought that `aioNMgrIob2_Create` returns a pointer to a
pointer. This may happen to work but it's not 100% accurate, so this has
been fixed by separating out the class and the vtable allocation.
Without this, the game could have accessed memory outside this buffer.

2. And then the SDVX implementation blindly copy-pasted the IIDX
`bi2x_hook`. Along with the issue above, they also forgot to check the
I/O DLLs to see how much memory needs to be allocated and change that.
Instead, they kept the same value as IIDX, which was too small. Fix that
as well.

## Testing
Doing some extensive testing with IIDX (27-33) and SDVX versions (EG).

src/spice2x/games/iidx/bi2x_hook.cpp

@@ -1,5 +1,7 @@
 #include "bi2x_hook.h"
 
+#if SPICE64
+
 #include <cstdint>
 #include "util/detour.h"
 #include "util/logging.h"
@@ -24,17 +26,28 @@ namespace games::iidx {
         uint8_t data[0x10];
     };
 
-    struct AIO_NMGR_IOB2 {
+    struct AIO_NMGR_IOB2_VTABLE {
-        uint8_t dummy0[0x50];
+        // other functions here, but they never get called
+        uint8_t dummy[0x50];
         void (__fastcall *pAIO_NMGR_IOB_BeginManage)(int64_t a1);
-        uint8_t dummy1[0x9A0];
     };
 
+    struct AIO_NMGR_IOB2 {
+        AIO_NMGR_IOB2_VTABLE *vptr;
+        uint8_t dummy[0x9F0];
+    };
+
+    // confirmed 0x9F8 in iidx27, iidx32 aioNMgrIob2_Create
+    static_assert(sizeof(AIO_NMGR_IOB2) == 0x9F8);
+
     struct AIO_IOB2_BI2X_TDJ {
         // who knows
         uint8_t data[0x13F8];
     };
 
+    // confirmed 0x13F8 in iidx27, iidx32 in aioIob2Bi2xTDJ_Create
+    static_assert(sizeof(AIO_IOB2_BI2X_TDJ) == 0x13F8);
+
     struct AIO_IOB2_BI2X_TDJ__DEVSTATUS {
         // of course you could work with variables here
         uint8_t buffer[0xCA];
@@ -78,7 +91,7 @@ namespace games::iidx {
     // libaio-iob.dll
     typedef AC_HNDLIF* (__fastcall *aioIob2Bi2x_OpenSciUsbCdc_t)(uint8_t device_num);
     typedef int64_t (__fastcall *aioIob2Bi2x_WriteFirmGetState_t)(int64_t a1);
-    typedef AIO_NMGR_IOB2** (__fastcall *aioNMgrIob2_Create_t)(AC_HNDLIF *a1, unsigned int a2);
+    typedef AIO_NMGR_IOB2* (__fastcall *aioNMgrIob2_Create_t)(AC_HNDLIF *a1, unsigned int a2);
 
     /*
      * function pointers
@@ -443,7 +456,7 @@ namespace games::iidx {
         }
     }
 
-    static AC_HNDLIF* __fastcall aioIob2Bi2X_OpenSciUsbCdc(uint8_t device_num) {
+    static AC_HNDLIF* __fastcall aioIob2Bi2x_OpenSciUsbCdc(uint8_t device_num) {
         if (acHndlif == nullptr) {
             acHndlif = new AC_HNDLIF;
             memset(acHndlif->data, 0x0, sizeof(acHndlif->data));
@@ -456,14 +469,16 @@ namespace games::iidx {
         log_info("bi2x_hook", "AIO_NMGR_IOB::BeginManage");
     };
 
-    static AIO_NMGR_IOB2** __fastcall aioNMgrIob2_Create(AC_HNDLIF *a1, unsigned int a2) {
+    static AIO_NMGR_IOB2* __fastcall aioNMgrIob2_Create(AC_HNDLIF *a1, unsigned int a2) {
         if (aioNmgrIob2 == nullptr) {
-            aioNmgrIob2 = new AIO_NMGR_IOB2;
+            aioNmgrIob2 = new AIO_NMGR_IOB2{};
-            memset(aioNmgrIob2, 0x0, sizeof(AIO_NMGR_IOB2));
+            aioNmgrIob2->vptr = new AIO_NMGR_IOB2_VTABLE{};
-            aioNmgrIob2->pAIO_NMGR_IOB_BeginManage = AIO_NMGR_IOB_BeginManageStub;
+            aioNmgrIob2->vptr->pAIO_NMGR_IOB_BeginManage = AIO_NMGR_IOB_BeginManageStub;
         }
-        log_info("bi2x_hook", "aioNMgrIob2_Create");
+        log_info("bi2x_hook", "aioNMgrIob2_Create returned {}, size=0x{:x}, vptr @ {}",
-        return &aioNmgrIob2;
+            fmt::ptr(aioNmgrIob2), sizeof(*aioNmgrIob2), fmt::ptr(aioNmgrIob2->vptr));
+
+        return aioNmgrIob2;
     }
 
     static int64_t __fastcall aioIob2Bi2x_WriteFirmGetState(int64_t a1) {
@@ -520,10 +535,12 @@ namespace games::iidx {
         // hook IOB
         const auto libaioIobDll = "libaio-iob.dll";
         detour::trampoline_try(libaioIobDll, "aioIob2Bi2x_OpenSciUsbCdc",
-                aioIob2Bi2X_OpenSciUsbCdc, &aioIob2Bi2x_OpenSciUsbCdc_orig);
+                aioIob2Bi2x_OpenSciUsbCdc, &aioIob2Bi2x_OpenSciUsbCdc_orig);
         detour::trampoline_try(libaioIobDll, "aioIob2Bi2x_WriteFirmGetState",
                 aioIob2Bi2x_WriteFirmGetState, &aioIob2Bi2x_WriteFirmGetState_orig);
         detour::trampoline_try(libaioIobDll, "aioNMgrIob2_Create",
                 aioNMgrIob2_Create, &aioNMgrIob2_Create_orig);
     }
 }
+
+#endif

src/spice2x/games/iidx/bi2x_hook.h

@@ -1,6 +1,10 @@
 #pragma once
 
+#if SPICE64
+
 namespace games::iidx {
 
     void bi2x_hook_init();
 }
+
+#endif

src/spice2x/games/iidx/iidx.cpp

@@ -251,6 +251,8 @@ namespace games::iidx {
         }
     }
 
+#if SPICE64
+
     typedef void* (*aioIob2Bi2x_CreateWriteFirmContext_t)(unsigned int, int);
     static aioIob2Bi2x_CreateWriteFirmContext_t aioIob2Bi2x_CreateWriteFirmContext_orig = nullptr;
 
@@ -269,6 +271,8 @@ namespace games::iidx {
         return nullptr;
     }
 
+#endif
+
     IIDXGame::IIDXGame() : Game("Beatmania IIDX") {
         logger::hook_add(log_hook, this);
     }
@@ -319,6 +323,8 @@ namespace games::iidx {
             devicehook_add(new IIDXFMSerialHandle());
         }
 
+#if SPICE64
+
         // check for new I/O DLL
         auto aio = libutils::try_library("libaio.dll");
         if (aio != nullptr) {
@@ -384,6 +390,8 @@ namespace games::iidx {
             }
         }
 
+#endif
+
         if (hooks::audio::ENABLED) {
             apply_audio_hacks();
         } else {

src/spice2x/games/sdvx/bi2x_hook.cpp

@@ -1,5 +1,7 @@
 #include "bi2x_hook.h"
 
+#if SPICE64
+
 #include <cstdint>
 #include "util/detour.h"
 #include "util/logging.h"
@@ -15,7 +17,6 @@
 
 namespace games::sdvx {
     constexpr bool BI2X_PASSTHROUGH = false;
-    bool BI2X_INITIALIZED = false;
 
     /*
      * class definitions
@@ -26,17 +27,28 @@ namespace games::sdvx {
         uint8_t data[0x10];
     };
 
-    struct AIO_NMGR_IOB2 {
+    struct AIO_NMGR_IOB2_VTABLE {
-        uint8_t dummy0[0x50];
+        // other functions here, but they never get called
+        uint8_t dummy[0x50];
         void (__fastcall *pAIO_NMGR_IOB_BeginManage)(int64_t a1);
-        uint8_t dummy1[0x9A0];
     };
 
+    struct AIO_NMGR_IOB2 {
+        AIO_NMGR_IOB2_VTABLE *vptr;
+        uint8_t dummy[0x9F0];
+    };
+
+    // confirmed in EG final in aioNMgrIob2_Create
+    static_assert(sizeof(AIO_NMGR_IOB2) == 0x9F8);
+
     struct AIO_IOB2_BI2X_UFC {
         // who knows
-        uint8_t data[0x13F8];
+        uint8_t data[0x39D8];
     };
 
+    // confirmed in EG final in aioIob2Bi2xUFC_Create
+    static_assert(sizeof(AIO_IOB2_BI2X_UFC) == 0x39D8);
+
     struct AIO_IOB2_BI2X_UFC__DEVSTATUS {
         // of course you could work with variables here
         uint8_t buffer[0x19E];
@@ -66,7 +78,7 @@ namespace games::sdvx {
     // libaio-iob.dll
     typedef AC_HNDLIF* (__fastcall *aioIob2Bi2x_OpenSciUsbCdc_t)(uint8_t device_num);
     typedef int64_t (__fastcall *aioIob2Bi2x_WriteFirmGetState_t)(int64_t a1);
-    typedef AIO_NMGR_IOB2** (__fastcall *aioNMgrIob2_Create_t)(AC_HNDLIF *a1, unsigned int a2);
+    typedef AIO_NMGR_IOB2* (__fastcall *aioNMgrIob2_Create_t)(AC_HNDLIF *a1, unsigned int a2);
 
     /*
      * function pointers
@@ -345,21 +357,21 @@ namespace games::sdvx {
         log_info("bi2x_hook", "AIO_NMGR_IOB::BeginManage");
     }
 
-    static AIO_NMGR_IOB2** __fastcall aioNMgrIob2_Create(AC_HNDLIF *a1, unsigned int a2) {
+    static AIO_NMGR_IOB2* __fastcall aioNMgrIob2_Create(AC_HNDLIF *a1, unsigned int a2) {
         if (aioNmgrIob2 == nullptr) {
-            aioNmgrIob2 = new AIO_NMGR_IOB2;
+            aioNmgrIob2 = new AIO_NMGR_IOB2{};
-            memset(aioNmgrIob2, 0x0, sizeof(AIO_NMGR_IOB2));
+            aioNmgrIob2->vptr = new AIO_NMGR_IOB2_VTABLE{};
-            aioNmgrIob2->pAIO_NMGR_IOB_BeginManage = AIO_NMGR_IOB_BeginManageStub;
+            aioNmgrIob2->vptr->pAIO_NMGR_IOB_BeginManage = AIO_NMGR_IOB_BeginManageStub;
         }
-        log_info("bi2x_hook", "aioNMgrIob2_Create");
+        log_info("bi2x_hook", "aioNMgrIob2_Create returned {}, size=0x{:x}, vptr @ {}",
-        BI2X_INITIALIZED = true;
+            fmt::ptr(aioNmgrIob2), sizeof(*aioNmgrIob2), fmt::ptr(aioNmgrIob2->vptr));
 
         // enable hack to make PIN pad work for KFC in BI2X mode
         // this explicit check in the I/O init path is necessary
         // (as opposed to just doing a check for "isValkyrieCabMode?")
         // because there are hex edits that allow you to use legacy (KFC/BIO2) IO while in Valk mode
         acioemu::ICCA_DEVICE_HACK = true;
-        return &aioNmgrIob2;
+        return aioNmgrIob2;
     }
 
     static int64_t __fastcall aioIob2Bi2x_WriteFirmGetState(int64_t a1) {
@@ -411,3 +423,5 @@ namespace games::sdvx {
                 aioNMgrIob2_Create, &aioNMgrIob2_Create_orig);
     }
 }
+
+#endif

src/spice2x/games/sdvx/bi2x_hook.h

@@ -1,6 +1,10 @@
 #pragma once
 
+#if SPICE64
+
 namespace games::sdvx {
 
     void bi2x_hook_init();
 }
+
+#endif

src/spice2x/games/sdvx/sdvx.h

@@ -20,7 +20,6 @@ namespace games::sdvx {
     extern bool NATIVETOUCH;
     extern uint8_t DIGITAL_KNOB_SENS;
     extern std::optional<std::string> ASIO_DRIVER;
-    extern bool BI2X_INITIALIZED;
     extern SdvxOverlayPosition OVERLAY_POS;
 
     // states