From d122492db7fab6f22ac6104789448cc4a65e2fec Mon Sep 17 00:00:00 2001
From: Amon Neander <138429719+MaverickAmon02@users.noreply.github.com>
Date: Thu, 6 Jul 2023 11:34:08 -0400
Subject: [PATCH] Fix unsafe netplay code in SI_DeviceGCController

By misusing Config, this netplay-related code opened up a race condition between Config::OnConfigChanged() and SerialInterface::SerialInterfaceManager::UpdateDevices() that could cause iterator invalidation.
---
 Source/Core/Core/HW/SI/SI.cpp                 |  2 +-
 Source/Core/Core/HW/SI/SI.h                   |  2 +-
 .../Core/Core/HW/SI/SI_DeviceGCController.cpp | 20 ++-----------------
 .../Core/Core/HW/SI/SI_DeviceGCController.h   |  7 -------
 4 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/Source/Core/Core/HW/SI/SI.cpp b/Source/Core/Core/HW/SI/SI.cpp
index 1312c548bc..401d963ddd 100644
--- a/Source/Core/Core/HW/SI/SI.cpp
+++ b/Source/Core/Core/HW/SI/SI.cpp
@@ -568,7 +568,7 @@ void SerialInterfaceManager::UpdateDevices()
   NetPlay::SetSIPollBatching(false);
 }
 
-SIDevices SerialInterfaceManager::GetDeviceType(int channel)
+SIDevices SerialInterfaceManager::GetDeviceType(int channel) const
 {
   if (channel < 0 || channel >= MAX_SI_CHANNELS || !m_channel[channel].device)
     return SIDEVICE_NONE;
diff --git a/Source/Core/Core/HW/SI/SI.h b/Source/Core/Core/HW/SI/SI.h
index cedd5856cd..9bd9f707a2 100644
--- a/Source/Core/Core/HW/SI/SI.h
+++ b/Source/Core/Core/HW/SI/SI.h
@@ -63,7 +63,7 @@ public:
 
   void ChangeDevice(SIDevices device, int channel);
 
-  SIDevices GetDeviceType(int channel);
+  SIDevices GetDeviceType(int channel) const;
 
   u32 GetPollXLines();
 
diff --git a/Source/Core/Core/HW/SI/SI_DeviceGCController.cpp b/Source/Core/Core/HW/SI/SI_DeviceGCController.cpp
index 598c84678b..26a5b03c47 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceGCController.cpp
+++ b/Source/Core/Core/HW/SI/SI_DeviceGCController.cpp
@@ -14,6 +14,7 @@
 #include "Core/CoreTiming.h"
 #include "Core/HW/GCPad.h"
 #include "Core/HW/ProcessorInterface.h"
+#include "Core/HW/SI/SI.h"
 #include "Core/HW/SI/SI_Device.h"
 #include "Core/HW/SystemTimers.h"
 #include "Core/Movie.h"
@@ -36,14 +37,6 @@ CSIDevice_GCController::CSIDevice_GCController(Core::System& system, SIDevices d
   m_origin.origin_stick_y = GCPadStatus::MAIN_STICK_CENTER_Y;
   m_origin.substick_x = GCPadStatus::C_STICK_CENTER_X;
   m_origin.substick_y = GCPadStatus::C_STICK_CENTER_Y;
-
-  m_config_changed_callback_id = Config::AddConfigChangedCallback([this] { RefreshConfig(); });
-  RefreshConfig();
-}
-
-CSIDevice_GCController::~CSIDevice_GCController()
-{
-  Config::RemoveConfigChangedCallback(m_config_changed_callback_id);
 }
 
 int CSIDevice_GCController::RunBuffer(u8* buffer, int request_length)
@@ -316,7 +309,7 @@ void CSIDevice_GCController::SendCommand(u32 command, u8 poll)
 
     if (pad_num < 4)
     {
-      const SIDevices device = m_config_si_devices[pad_num];
+      const SIDevices device = m_system.GetSerialInterface().GetDeviceType(pad_num);
       if (type == 1)
         CSIDevice_GCController::Rumble(pad_num, 1.0, device);
       else
@@ -346,15 +339,6 @@ void CSIDevice_GCController::DoState(PointerWrap& p)
   p.Do(m_last_button_combo);
 }
 
-void CSIDevice_GCController::RefreshConfig()
-{
-  for (int i = 0; i < 4; ++i)
-  {
-    const SerialInterface::SIDevices device = Config::Get(Config::GetInfoForSIDevice(i));
-    m_config_si_devices[i] = device;
-  }
-}
-
 CSIDevice_TaruKonga::CSIDevice_TaruKonga(Core::System& system, SIDevices device, int device_number)
     : CSIDevice_GCController(system, device, device_number)
 {
diff --git a/Source/Core/Core/HW/SI/SI_DeviceGCController.h b/Source/Core/Core/HW/SI/SI_DeviceGCController.h
index 809eaabd3b..43e3d7aabb 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceGCController.h
+++ b/Source/Core/Core/HW/SI/SI_DeviceGCController.h
@@ -54,7 +54,6 @@ protected:
 public:
   // Constructor
   CSIDevice_GCController(Core::System& system, SIDevices device, int device_number);
-  ~CSIDevice_GCController() override;
 
   // Run the SI Buffer
   int RunBuffer(u8* buffer, int request_length) override;
@@ -83,12 +82,6 @@ public:
 
 protected:
   void SetOrigin(const GCPadStatus& pad_status);
-
-private:
-  void RefreshConfig();
-
-  std::array<SIDevices, 4> m_config_si_devices{};
-  size_t m_config_changed_callback_id;
 };
 
 // "TaruKonga", the DK Bongo controller