From 310daa61a8e30b29be0eccfbcdc7ec67b6c05a17 Mon Sep 17 00:00:00 2001 From: Frank Date: Wed, 21 Jun 2023 01:43:26 +0200 Subject: [PATCH] improve segment code robustness * Avoid uint16 underflow in width() and height(): stop > start is possible, and means "inactive segment". * use size_t for ledsrgbSize, _dataLen and _usedSegmentData - uint16_t could overflow in some situations. * try to catch attempts to allocate zero bytes (inactive segment => size 0) --- wled00/FX.h | 29 ++++++++++++++++------------- wled00/FX_2Dfcn.cpp | 8 ++++---- wled00/FX_fcn.cpp | 28 ++++++++++++++++++---------- wled00/wled.h | 4 ++-- 4 files changed, 40 insertions(+), 29 deletions(-) diff --git a/wled00/FX.h b/wled00/FX.h index 732af8ff..e12b03c3 100644 --- a/wled00/FX.h +++ b/wled00/FX.h @@ -31,6 +31,7 @@ #include "const.h" +bool canUseSerial(void); // WLEDMM implemented in wled_serial.cpp void strip_wait_until_idle(String whoCalledMe); // WLEDMM implemented in FX_fcn.cpp #define FASTLED_INTERNAL //remove annoying pragma messages @@ -398,7 +399,7 @@ typedef struct Segment { uint16_t aux1; // custom var byte* data; // effect data pointer CRGB* ledsrgb; // local leds[] array (may be a pointer to global) //WLEDMM rename to ledsrgb to search on them (temp?) - uint16_t ledsrgbSize; //WLEDMM + size_t ledsrgbSize; //WLEDMM static CRGB *_globalLeds; // global leds[] array static uint16_t maxWidth, maxHeight; // these define matrix width & height (max. segment dimensions) void *jMap = nullptr; //WLEDMM jMap @@ -414,8 +415,8 @@ typedef struct Segment { uint8_t _reserved : 4; }; }; - uint16_t _dataLen; - static uint16_t _usedSegmentData; + size_t _dataLen; // WLEDMM uint16_t is too small + static size_t _usedSegmentData; // WLEDMM uint16_t is too small // transition data, valid only if transitional==true, holds values during transition struct Transition { @@ -502,11 +503,13 @@ typedef struct Segment { ~Segment() { //#ifdef WLED_DEBUG - Serial.print(F("Destroying segment:")); - if (name) Serial.printf(" %s (%p)", name, name); - if (data) Serial.printf(" %d (%p)", (int)_dataLen, data); - if (ledsrgb) Serial.printf(" [%u]", length()*sizeof(CRGB)); - Serial.println(); + if(canUseSerial()) { + Serial.print(F("Destroying segment:")); + if (name) Serial.printf(" name=%s (%p)", name, name); + if (data) Serial.printf(" dataLen=%d (%p)", (int)_dataLen, data); + if (ledsrgb) Serial.printf(" [%sledsrgb %u bytes]", Segment::_globalLeds ? "global ":"",length()*sizeof(CRGB)); + Serial.println(); + } //#endif // WLEDMM only delete segments when they are not in use @@ -514,7 +517,7 @@ typedef struct Segment { strip_wait_until_idle("~Segment()"); #endif - if (!Segment::_globalLeds && ledsrgb) free(ledsrgb); + if (!Segment::_globalLeds && (ledsrgb != nullptr)) {free(ledsrgb); ledsrgb = nullptr;} if (name) delete[] name; if (_t) delete _t; deallocateData(); @@ -534,13 +537,13 @@ typedef struct Segment { inline bool hasRGB(void) const { return _isRGB; } inline bool hasWhite(void) const { return _hasW; } inline bool isCCT(void) const { return _isCCT; } - inline uint16_t width(void) const { return stop - start; } // segment width in physical pixels (length if 1D) - inline uint16_t height(void) const { return max(1, stopY - startY); } // segment height (if 2D) in physical pixels // WLEDMM make sure its always > 0 + inline uint16_t width(void) const { return (stop > start) ? (stop - start) : 0; } // segment width in physical pixels (length if 1D) + inline uint16_t height(void) const { return (stopY > startY) ? (stopY - startY) : 0; } // segment height (if 2D) in physical pixels // WLEDMM make sure its always > 0 inline uint16_t length(void) const { return width() * height(); } // segment length (count) in physical pixels inline uint16_t groupLength(void) const { return grouping + spacing; } inline uint8_t getLightCapabilities(void) const { return _capabilities; } - static uint16_t getUsedSegmentData(void) { return _usedSegmentData; } + static size_t getUsedSegmentData(void) { return _usedSegmentData; } // WLEDMM size_t static void addUsedSegmentData(int len) { _usedSegmentData += len; } void allocLeds(); //WLEDMM @@ -556,7 +559,7 @@ typedef struct Segment { void refreshLightCapabilities(void); // runtime data functions - inline uint16_t dataSize(void) const { return _dataLen; } + inline size_t dataSize(void) const { return _dataLen; } bool allocateData(size_t len); void deallocateData(void); void resetIfRequired(void); diff --git a/wled00/FX_2Dfcn.cpp b/wled00/FX_2Dfcn.cpp index f746a1bf..636ed4f7 100644 --- a/wled00/FX_2Dfcn.cpp +++ b/wled00/FX_2Dfcn.cpp @@ -67,16 +67,16 @@ void WS2812FX::setUpMatrix() { //WLEDMM recreate customMappingTable if more space needed if (Segment::maxWidth * Segment::maxHeight > customMappingTableSize) { - uint32_t size = MAX(ledmapMaxSize, Segment::maxWidth * Segment::maxHeight);//TroyHack + size_t size = max(ledmapMaxSize, size_t(Segment::maxWidth * Segment::maxHeight));//TroyHack USER_PRINTF("setupmatrix customMappingTable alloc %d from %d\n", size, customMappingTableSize); //if (customMappingTable != nullptr) delete[] customMappingTable; //customMappingTable = new uint16_t[size]; // don't use new / delete - if (customMappingTable != nullptr) { // resize + if ((size > 0) && (customMappingTable != nullptr)) { // resize customMappingTable = (uint16_t*) reallocf(customMappingTable, sizeof(uint16_t) * size); // reallocf will free memory if it cannot resize } - if (customMappingTable == nullptr) { // second try + if ((size > 0) && (customMappingTable == nullptr)) { // second try DEBUG_PRINTLN("setUpMatrix: trying to get fresh memory block."); customMappingTable = (uint16_t*) calloc(size, sizeof(uint16_t)); if (customMappingTable == nullptr) DEBUG_PRINTLN("setUpMatrix: alloc failed"); @@ -144,7 +144,7 @@ void WS2812FX::setUpMatrix() { } // delete gap array as we no longer need it - if (gapTable) delete[] gapTable; + if (gapTable) {delete[] gapTable; gapTable=nullptr;} // softhack prevent dangling pointer #ifdef WLED_DEBUG_MAPS DEBUG_PRINTF("Matrix ledmap: \n"); diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp index c170f31d..769ef356 100644 --- a/wled00/FX_fcn.cpp +++ b/wled00/FX_fcn.cpp @@ -87,7 +87,7 @@ void strip_wait_until_idle(String whoCalledMe) { /////////////////////////////////////////////////////////////////////////////// // Segment class implementation /////////////////////////////////////////////////////////////////////////////// -uint16_t Segment::_usedSegmentData = 0U; // amount of RAM all segments use for their data[] +size_t Segment::_usedSegmentData = 0U; // amount of RAM all segments use for their data[] CRGB *Segment::_globalLeds = nullptr; uint16_t Segment::maxWidth = DEFAULT_LED_COUNT; uint16_t Segment::maxHeight = 1; @@ -110,14 +110,22 @@ Segment::Segment(const Segment &orig) { //WLEDMM: recreate ledsrgb if more space needed (will not free ledsrgb!) void Segment::allocLeds() { - uint16_t size = sizeof(CRGB)*MAX(length(), ledmapMaxSize); //TroyHack - if (!ledsrgb || size > ledsrgbSize) { - USER_PRINTF("allocLeds %d from %d\n", size, ledsrgb?ledsrgbSize:0); + size_t size = sizeof(CRGB)*max((size_t) length(), ledmapMaxSize); //TroyHack + if ((size < sizeof(CRGB)) || (size > 164000)) { //softhack too small (<3) or too large (>160Kb) + USER_PRINTF("allocLeds warning: size == %u !!\n", size); + USER_FLUSH(); + if (ledsrgb && (ledsrgbSize == 0)) { + USER_PRINTLN("allocLeds warning: ledsrgbSize == 0 but ledsrgb!=NULL"); + free(ledsrgb); ledsrgb=nullptr; + } // softhack007 clean up buffer + } + if ((size > 0) && (!ledsrgb || size > ledsrgbSize)) { //softhack dont allocate zero bytes + USER_PRINTF("allocLeds %u from %u\n", size, ledsrgb?ledsrgbSize:0); ledsrgb = (CRGB*)malloc(size); ledsrgbSize = ledsrgb?size:0; } else { - USER_PRINTF("reuse Leds %d from %d\n", size, ledsrgb?ledsrgbSize:0); + USER_PRINTF("reuse Leds %u from %u\n", size, ledsrgb?ledsrgbSize:0); } } @@ -143,7 +151,7 @@ Segment& Segment::operator= (const Segment &orig) { // WLEDMM reuse leds instead of removing themn // if (ledsrgb && !Segment::_globalLeds) free(ledsrgb); //WLEDMM: nullify below! CRGB* oldLeds = ledsrgb; - uint16_t oldLedsSize = ledsrgbSize; + size_t oldLedsSize = ledsrgbSize; deallocateData(); // copy source memcpy((void*)this, (void*)&orig, sizeof(Segment)); @@ -2249,16 +2257,16 @@ bool WS2812FX::deserializeMap(uint8_t n) { //WLEDMM recreate customMappingTable if more space needed if (Segment::maxWidth * Segment::maxHeight > customMappingTableSize) { - uint32_t size = MAX(ledmapMaxSize, Segment::maxWidth * Segment::maxHeight);//TroyHack - USER_PRINTF("deserializemap customMappingTable alloc %d from %d\n", size, customMappingTableSize); + size_t size = max(ledmapMaxSize, size_t(Segment::maxWidth * Segment::maxHeight));//TroyHack + USER_PRINTF("deserializemap customMappingTable alloc %u from %u\n", size, customMappingTableSize); //if (customMappingTable != nullptr) delete[] customMappingTable; //customMappingTable = new uint16_t[size]; // don't use new / delete - if (customMappingTable != nullptr) { + if ((size > 0) && (customMappingTable != nullptr)) { customMappingTable = (uint16_t*) reallocf(customMappingTable, sizeof(uint16_t) * size); // reallocf will free memory if it cannot resize } - if (customMappingTable == nullptr) { // second try + if ((size > 0) && (customMappingTable == nullptr)) { // second try DEBUG_PRINTLN("deserializeMap: trying to get fresh memory block."); customMappingTable = (uint16_t*) calloc(size, sizeof(uint16_t)); if (customMappingTable == nullptr) DEBUG_PRINTLN("deserializeMap: alloc failed!"); diff --git a/wled00/wled.h b/wled00/wled.h index 59fce6a3..0a0402d9 100644 --- a/wled00/wled.h +++ b/wled00/wled.h @@ -8,7 +8,7 @@ */ // version code in format yymmddb (b = daily build) -#define VERSION 2306160 +#define VERSION 2306210 //uncomment this if you have a "my_config.h" file you'd like to use //#define WLED_USE_MY_CONFIG @@ -703,7 +703,7 @@ WLED_GLOBAL volatile bool suspendStripService _INIT(false); // WLEDMM temporaril #ifndef ESP8266 WLED_GLOBAL char *ledmapNames[WLED_MAX_LEDMAPS-1] _INIT_N(({nullptr})); #endif -WLED_GLOBAL uint16_t ledmapMaxSize _INIT(0); //WLEDMM TroyHack +WLED_GLOBAL size_t ledmapMaxSize _INIT(0); //WLEDMM TroyHack #if WLED_MAX_LEDMAPS>16 WLED_GLOBAL uint32_t ledMaps _INIT(0); // bitfield representation of available ledmaps #else