From 6a134a1f759ed655eb2ba38ef31832fae11b397c Mon Sep 17 00:00:00 2001 From: TheRogueZeta <6479694-TheRogueZeta@users.noreply.gitlab.com> Date: Mon, 21 Jun 2021 04:45:48 +0000 Subject: [PATCH] Add logging to ASRock Polychome SMBus detection and fix stack smashing caused by Polychrome block read. --- .../ASRockPolychromeSMBusController.cpp | 35 +++++++++++++------ .../ASRockPolychromeSMBusController.h | 1 + .../ASRockPolychromeSMBusControllerDetect.cpp | 4 +++ .../RGBController_ASRockPolychromeSMBus.cpp | 2 +- 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/Controllers/ASRockPolychromeSMBusController/ASRockPolychromeSMBusController.cpp b/Controllers/ASRockPolychromeSMBusController/ASRockPolychromeSMBusController.cpp index 9323ee7a..24447edf 100644 --- a/Controllers/ASRockPolychromeSMBusController/ASRockPolychromeSMBusController.cpp +++ b/Controllers/ASRockPolychromeSMBusController/ASRockPolychromeSMBusController.cpp @@ -10,6 +10,7 @@ #include "ASRockPolychromeSMBusController.h" #include #include "dependencies/dmiinfo.h" +#include "LogManager.h" using namespace std::chrono_literals; @@ -20,36 +21,39 @@ PolychromeController::PolychromeController(i2c_smbus_interface* bus, polychrome_ DMIInfo dmi; - unsigned short fw_version = ReadFirmwareVersion(); - unsigned char major_version = fw_version >> 8; + device_name = "ASRock " + dmi.getMainboard(); + fw_version = ReadFirmwareVersion(); + unsigned char major_version = fw_version >> 8; /*-----------------------------------------------------*\ | Determine whether the device uses ASR LED or | | Polychrome protocol by checking firmware version. | - | Versions 1.xx and 2.xx use ASR LED, 3.xx uses | - | Polychrome | + | Versions: 1.xx are ASR RGB LED | + | 2.xx are Polychrome v1 | + | 3.xx are Polychrome v2 | \*-----------------------------------------------------*/ switch(major_version) { case ASROCK_TYPE_ASRLED: - device_name = "ASRock " + dmi.getMainboard(); + LOG_DEBUG("Device type is ASR RGB LED"); asrock_type = ASROCK_TYPE_ASRLED; memset(zone_led_count, 0, sizeof(zone_led_count)); break; case ASROCK_TYPE_POLYCHROME_V1: - device_name = "ASRock " + dmi.getMainboard(); + LOG_DEBUG("Device type is Polychrome v1"); asrock_type = ASROCK_TYPE_POLYCHROME_V1; ReadLEDConfiguration(); break; case ASROCK_TYPE_POLYCHROME_V2: - device_name = "ASRock " + dmi.getMainboard(); + LOG_DEBUG("Device type is Polychrome v2"); asrock_type = ASROCK_TYPE_POLYCHROME_V2; ReadLEDConfiguration(); break; default: + LOG_DEBUG("Got Unknown version!"); asrock_type = ASROCK_TYPE_UNKNOWN; break; } @@ -82,7 +86,6 @@ std::string PolychromeController::GetDeviceName() std::string PolychromeController::GetFirmwareVersion() { - unsigned short fw_version = ReadFirmwareVersion(); unsigned char major_version = fw_version >> 8; unsigned char minor_version = fw_version & 0xFF; @@ -93,16 +96,20 @@ unsigned short PolychromeController::ReadFirmwareVersion() { // The firmware register holds two bytes, so the first read should return 2 // If not, report invalid firmware revision FFFF - unsigned char asrock_version[2] = { 0x00, 0x00}; + LOG_DEBUG("Reading back device firmware version"); + // Version response array needs to be 32 bytes to prevent non ASRock board from stack smashing + unsigned char asrock_version[I2C_SMBUS_BLOCK_MAX] = { 0x00, 0x00 }; if (bus->i2c_smbus_read_block_data(dev, ASROCK_REG_FIRMWARE_VER, asrock_version) == 0x02) { unsigned char major = asrock_version[0]; unsigned char minor = asrock_version[1]; + LOG_DEBUG("Device firmware version: v%02d.%02d", major, minor); return((major << 8) | minor); } else { + LOG_WARNING("Firmware readback failed; Returning 0xFFFF"); return(0xFFFF); } } @@ -113,7 +120,8 @@ void PolychromeController::ReadLEDConfiguration() | The LED configuration register holds 6 bytes, so the first read should return 6 | | If not, set all zone sizes to zero | \*---------------------------------------------------------------------------------*/ - unsigned char asrock_zone_count[6] = { 0x0 }; + LOG_DEBUG("Reading back LED config"); + unsigned char asrock_zone_count[I2C_SMBUS_BLOCK_MAX] = { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 }; if (bus->i2c_smbus_read_block_data(dev, POLYCHROME_REG_LED_CONFIG, asrock_zone_count) == 0x06) { zone_led_count[POLYCHROME_ZONE_1] = asrock_zone_count[0]; @@ -122,9 +130,16 @@ void PolychromeController::ReadLEDConfiguration() zone_led_count[POLYCHROME_ZONE_4] = asrock_zone_count[3]; zone_led_count[POLYCHROME_ZONE_5] = asrock_zone_count[4]; zone_led_count[POLYCHROME_ZONE_ADDRESSABLE] = asrock_zone_count[5]; + LOG_DEBUG("Zone 1 LED count: %02d", zone_led_count[POLYCHROME_ZONE_1]); + LOG_DEBUG("Zone 2 LED count: %02d", zone_led_count[POLYCHROME_ZONE_2]); + LOG_DEBUG("Zone 3 LED count: %02d", zone_led_count[POLYCHROME_ZONE_3]); + LOG_DEBUG("Zone 4 LED count: %02d", zone_led_count[POLYCHROME_ZONE_4]); + LOG_DEBUG("Zone 5 LED count: %02d", zone_led_count[POLYCHROME_ZONE_5]); + LOG_DEBUG("Addressable Zone LED count: %02d", zone_led_count[POLYCHROME_ZONE_ADDRESSABLE]); } else { + LOG_WARNING("LED config read failed"); memset(zone_led_count, 0, sizeof(zone_led_count)); } } diff --git a/Controllers/ASRockPolychromeSMBusController/ASRockPolychromeSMBusController.h b/Controllers/ASRockPolychromeSMBusController/ASRockPolychromeSMBusController.h index b568cb2f..00f7b32d 100644 --- a/Controllers/ASRockPolychromeSMBusController/ASRockPolychromeSMBusController.h +++ b/Controllers/ASRockPolychromeSMBusController/ASRockPolychromeSMBusController.h @@ -209,6 +209,7 @@ public: private: unsigned int asrock_type; + unsigned short fw_version; std::string device_name; unsigned char active_zone; unsigned char active_mode; diff --git a/Controllers/ASRockPolychromeSMBusController/ASRockPolychromeSMBusControllerDetect.cpp b/Controllers/ASRockPolychromeSMBusController/ASRockPolychromeSMBusControllerDetect.cpp index b8532735..e9e0fd7b 100644 --- a/Controllers/ASRockPolychromeSMBusController/ASRockPolychromeSMBusControllerDetect.cpp +++ b/Controllers/ASRockPolychromeSMBusController/ASRockPolychromeSMBusControllerDetect.cpp @@ -1,5 +1,6 @@ #include "Detector.h" #include "ASRockPolychromeSMBusController.h" +#include "LogManager.h" #include "RGBController.h" #include "RGBController_ASRockPolychromeSMBus.h" #include "i2c_smbus.h" @@ -56,15 +57,18 @@ void DetectPolychromeSMBusControllers(std::vector& busses) // Check for Polychrome controller at 0x6A if (TestForPolychromeSMBusController(busses[bus], 0x6A)) { + LOG_DEBUG("Detected a device at address 0x6A, Testing for a known controller"); new_polychrome = new PolychromeController(busses[bus], 0x6A); if(new_polychrome->GetASRockType() != ASROCK_TYPE_UNKNOWN) { + LOG_DEBUG("Found a known Polychrome device"); new_controller = new RGBController_Polychrome(new_polychrome); ResourceManager::get()->RegisterRGBController(new_controller); } else { + LOG_DEBUG("Not a Polychrome device or unknown type"); delete new_polychrome; } } diff --git a/Controllers/ASRockPolychromeSMBusController/RGBController_ASRockPolychromeSMBus.cpp b/Controllers/ASRockPolychromeSMBusController/RGBController_ASRockPolychromeSMBus.cpp index 5d556dc4..fff7602f 100644 --- a/Controllers/ASRockPolychromeSMBusController/RGBController_ASRockPolychromeSMBus.cpp +++ b/Controllers/ASRockPolychromeSMBusController/RGBController_ASRockPolychromeSMBus.cpp @@ -46,7 +46,7 @@ RGBController_Polychrome::RGBController_Polychrome(PolychromeController* polychr { case ASROCK_TYPE_ASRLED: { - description = "ASRock ASR LED Device"; + description = "ASRock ASR RGB LED Device"; mode Off; Off.name = "Off";