pinmanager robustness improvement

make sure that array bounds are not violated in pinManager class.
This commit is contained in:
Frank 2022-12-05 17:04:54 +01:00
parent da7972c119
commit e7f07f5bfc
2 changed files with 12 additions and 4 deletions

View File

@ -107,8 +107,9 @@ bool PinManagerClass::allocateMultiplePins(const managed_pin_type * mptArray, by
} }
if (!isPinOk(gpio, mptArray[i].isOutput)) { if (!isPinOk(gpio, mptArray[i].isOutput)) {
#ifdef WLED_DEBUG #ifdef WLED_DEBUG
DEBUG_PRINT(F("PIN ALLOC: Invalid pin attempted to be allocated: ")); DEBUG_PRINT(F("PIN ALLOC: Invalid pin attempted to be allocated: GPIO "));
DEBUG_PRINT(gpio); DEBUG_PRINT(gpio);
DEBUG_PRINT(" as "); DEBUG_PRINT(mptArray[i].isOutput ? "output": "input");
DEBUG_PRINTLN(F("")); DEBUG_PRINTLN(F(""));
#endif #endif
shouldFail = true; shouldFail = true;
@ -142,6 +143,9 @@ bool PinManagerClass::allocateMultiplePins(const managed_pin_type * mptArray, by
// as this can greatly simplify configuration arrays // as this can greatly simplify configuration arrays
continue; continue;
} }
if (gpio >= WLED_NUM_PINS)
continue; // other unexpected GPIO => avoid array bounds violation
byte by = gpio >> 3; byte by = gpio >> 3;
byte bi = gpio - 8*by; byte bi = gpio - 8*by;
bitWrite(pinAlloc[by], bi, true); bitWrite(pinAlloc[by], bi, true);
@ -160,7 +164,7 @@ bool PinManagerClass::allocateMultiplePins(const managed_pin_type * mptArray, by
bool PinManagerClass::allocatePin(byte gpio, bool output, PinOwner tag) bool PinManagerClass::allocatePin(byte gpio, bool output, PinOwner tag)
{ {
// HW I2C & SPI pins have to be allocated using allocateMultiplePins variant since there is always SCL/SDA pair // HW I2C & SPI pins have to be allocated using allocateMultiplePins variant since there is always SCL/SDA pair
if (!isPinOk(gpio, output) || tag==PinOwner::HW_I2C || tag==PinOwner::HW_SPI) { if (!isPinOk(gpio, output) || (gpio >= WLED_NUM_PINS) || tag==PinOwner::HW_I2C || tag==PinOwner::HW_SPI) {
#ifdef WLED_DEBUG #ifdef WLED_DEBUG
if (gpio < 255) { // 255 (-1) is the "not defined GPIO" if (gpio < 255) { // 255 (-1) is the "not defined GPIO"
if (!isPinOk(gpio, output)) { if (!isPinOk(gpio, output)) {
@ -209,6 +213,7 @@ bool PinManagerClass::isPinAllocated(byte gpio, PinOwner tag)
{ {
if (!isPinOk(gpio, false)) return true; if (!isPinOk(gpio, false)) return true;
if ((tag != PinOwner::None) && (ownerTag[gpio] != tag)) return false; if ((tag != PinOwner::None) && (ownerTag[gpio] != tag)) return false;
if (gpio >= WLED_NUM_PINS) return false; // catch error case, to avoid array out-of-bounds access
byte by = gpio >> 3; byte by = gpio >> 3;
byte bi = gpio - (by<<3); byte bi = gpio - (by<<3);
return bitRead(pinAlloc[by], bi); return bitRead(pinAlloc[by], bi);
@ -265,6 +270,7 @@ bool PinManagerClass::isPinOk(byte gpio, bool output)
} }
PinOwner PinManagerClass::getPinOwner(byte gpio) { PinOwner PinManagerClass::getPinOwner(byte gpio) {
if (gpio >= WLED_NUM_PINS) return PinOwner::None; // catch error case, to avoid array out-of-bounds access
if (!isPinOk(gpio, false)) return PinOwner::None; if (!isPinOk(gpio, false)) return PinOwner::None;
return ownerTag[gpio]; return ownerTag[gpio];
} }

View File

@ -66,12 +66,14 @@ static_assert(0u == static_cast<uint8_t>(PinOwner::None), "PinOwner::None must b
class PinManagerClass { class PinManagerClass {
private: private:
#ifdef ESP8266 #ifdef ESP8266
#define WLED_NUM_PINS 17
uint8_t pinAlloc[3] = {0x00, 0x00, 0x00}; //24bit, 1 bit per pin, we use first 17bits uint8_t pinAlloc[3] = {0x00, 0x00, 0x00}; //24bit, 1 bit per pin, we use first 17bits
PinOwner ownerTag[17] = { PinOwner::None }; PinOwner ownerTag[WLED_NUM_PINS] = { PinOwner::None };
#else #else
#define WLED_NUM_PINS 50
uint8_t pinAlloc[7] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; // 56bit, 1 bit per pin, we use 50 bits on ESP32-S3 uint8_t pinAlloc[7] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; // 56bit, 1 bit per pin, we use 50 bits on ESP32-S3
uint8_t ledcAlloc[2] = {0x00, 0x00}; //16 LEDC channels uint8_t ledcAlloc[2] = {0x00, 0x00}; //16 LEDC channels
PinOwner ownerTag[50] = { PinOwner::None }; // new MCU's have up to 50 GPIO PinOwner ownerTag[WLED_NUM_PINS] = { PinOwner::None }; // new MCU's have up to 50 GPIO
#endif #endif
struct { struct {
uint8_t i2cAllocCount : 4; // allow multiple allocation of I2C bus pins but keep track of allocations uint8_t i2cAllocCount : 4; // allow multiple allocation of I2C bus pins but keep track of allocations