ElixirForge/firmware
1
0
Fork 0
mirror of https://github.com/alexhopeoconnor/firmware.git synced 2026-10-03 19:12:00 +10:00
Code Releases Activity

fix(graphics): crash and leak fixes across display drivers (#11455)

Browse Source 
* fix(graphics): crash and leak fixes across display drivers

- TFTDisplay (portduino): _touch_instance was an uninitialized member,
  and the touch-config block only assigns it for xpt2046/stmpe610/
  ft5x06 while the guard accepts any configured module. A gt911 entry
  in config.yaml (supported by the color-UI path) reached
  _touch_instance->config() through an indeterminate pointer. Initialize
  to nullptr and guard the config block.

- Screen: the destructor freed normalFrames but leaked the owned
  dispdev (driver + framebuffer) and ui objects. Screen is genuinely
  destroyed on the portduino reboot path (screen = nullptr in
  Power.cpp).

- EInkDisplay2: GxEPD2_BW's constructor takes the low-level driver by
  value and stores a copy, so the 'new EINK_DISPLAY_MODEL' at nine
  sites was orphaned the moment connect() returned. Pass temporaries,
  as GxEPD2Multi already does.

- EInkParallelDisplay: the async full-refresh task cleared
  asyncFullRunning before nulling asyncTaskHandle, so the destructor
  could observe running==false with a stale handle and vTaskDelete a
  freed TCB. Null the handle first (same ordering fix the
  eink/Drivers/EInkParallel.cpp sibling already carries).

- Panel_sdl: initFrameBuffer only null-checked the first of its three
  allocations and returned true regardless, leaving the line array
  full of null+offset garbage on failure; later redraws would write
  through those. Check all three, release partial allocations, and
  return false.

* fix(graphics): propagate Panel_sdl framebuffer allocation failure from init()

Per review: initFrameBuffer() can now fail cleanly, so init() must not
register the monitor and report success when it does.
This commit is contained in:
Ben Meadors
2026-08-18 12:10:03 +00:00
committed by GitHub
parent c308d0aca4
commit e1ea653a45
5 changed files with 72 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/graphics/EInkDisplay2.cpp
+27 -23
View File
@@ -161,9 +161,9 @@ bool EInkDisplay::connect()
#if defined(TTGO_T_ECHO) || defined(ELECROW_ThinkNode_M1) || defined(T_ECHO_LITE) || defined(TTGO_T_ECHO_PLUS) || \
defined(ELECROW_ThinkNode_M8)
{
auto lowLevel = new EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY, SPI1);
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(*lowLevel);
// GxEPD2_BW stores a copy of the driver, so pass a temporary instead of leaking a heap object
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(
EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY, SPI1));
adafruitDisplay->init();
#if defined(ELECROW_ThinkNode_M1) || defined(T_ECHO_LITE) || defined(ELECROW_ThinkNode_M8)
adafruitDisplay->setRotation(4);
@@ -178,9 +178,9 @@ bool EInkDisplay::connect()
hspi = new SPIClass(HSPI);
hspi->begin(PIN_EINK_SCLK, -1, PIN_EINK_MOSI, PIN_EINK_CS); // SCLK, MISO, MOSI, SS
auto lowLevel = new EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY, *hspi);
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(*lowLevel);
// GxEPD2_BW stores a copy of the driver, so pass a temporary instead of leaking a heap object
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(
EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY, *hspi));
adafruitDisplay->init();
adafruitDisplay->setRotation(4);
@@ -189,9 +189,9 @@ bool EInkDisplay::connect()
}
#elif defined(MESHLINK)
{
auto lowLevel = new EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY, SPI1);
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(*lowLevel);
// GxEPD2_BW stores a copy of the driver, so pass a temporary instead of leaking a heap object
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(
EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY, SPI1));
adafruitDisplay->init();
adafruitDisplay->setRotation(3);
adafruitDisplay->setPartialWindow(0, 0, displayWidth, displayHeight);
@@ -199,8 +199,9 @@ bool EInkDisplay::connect()
#elif defined(RAK4630) || defined(MAKERPYTHON)
{
if (eink_found) {
auto lowLevel = new EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY);
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(*lowLevel);
// GxEPD2_BW stores a copy of the driver, so pass a temporary instead of leaking a heap object
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(
EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY));
adafruitDisplay->init(115200, true, 10, false, SPI1, SPISettings(4000000, MSBFIRST, SPI_MODE0));
// RAK14000 2.13 inch b/w 250x122 does actually now support fast refresh
adafruitDisplay->setRotation(3);
@@ -236,9 +237,9 @@ bool EInkDisplay::connect()
// VExt already enabled in setup()
// RTC GPIO hold disabled in setup()
// Create GxEPD2 objects
auto lowLevel = new EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY, *hspi);
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(*lowLevel);
// Create GxEPD2 objects (GxEPD2_BW stores a copy of the driver, so pass a temporary)
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(
EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY, *hspi));
// Init GxEPD2
adafruitDisplay->init();
@@ -253,22 +254,25 @@ bool EInkDisplay::connect()
}
#elif defined(PCA10059) || defined(ME25LS01)
{
auto lowLevel = new EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY);
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(*lowLevel);
// GxEPD2_BW stores a copy of the driver, so pass a temporary instead of leaking a heap object
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(
EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY));
adafruitDisplay->init(115200, true, 40, false, SPI1, SPISettings(4000000, MSBFIRST, SPI_MODE0));
adafruitDisplay->setRotation(0);
adafruitDisplay->setPartialWindow(0, 0, EINK_WIDTH, EINK_HEIGHT);
}
#elif defined(M5_COREINK) || defined(T_DECK_PRO)
auto lowLevel = new EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY);
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(*lowLevel);
// GxEPD2_BW stores a copy of the driver, so pass a temporary instead of leaking a heap object
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(
EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY));
adafruitDisplay->init(115200, true, 40, false, SPI, SPISettings(4000000, MSBFIRST, SPI_MODE0));
adafruitDisplay->setRotation(0);
adafruitDisplay->setPartialWindow(0, 0, EINK_WIDTH, EINK_HEIGHT);
#elif defined(my) || defined(ESP32_S3_PICO)
{
auto lowLevel = new EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY);
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(*lowLevel);
// GxEPD2_BW stores a copy of the driver, so pass a temporary instead of leaking a heap object
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(
EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY));
adafruitDisplay->init(115200, true, 40, false, SPI, SPISettings(4000000, MSBFIRST, SPI_MODE0));
adafruitDisplay->setRotation(1);
adafruitDisplay->setPartialWindow(0, 0, EINK_WIDTH, EINK_HEIGHT);
@@ -280,9 +284,9 @@ bool EInkDisplay::connect()
// VExt already enabled in setup()
// RTC GPIO hold disabled in setup()
// Create GxEPD2 objects
auto lowLevel = new EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY, *spi1);
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(*lowLevel);
// Create GxEPD2 objects (GxEPD2_BW stores a copy of the driver, so pass a temporary)
adafruitDisplay = new GxEPD2_BW<EINK_DISPLAY_MODEL, EINK_DISPLAY_MODEL::HEIGHT>(
EINK_DISPLAY_MODEL(PIN_EINK_CS, PIN_EINK_DC, PIN_EINK_RES, PIN_EINK_BUSY, *spi1));
// Init GxEPD2
adafruitDisplay->init();
src/graphics/EInkParallelDisplay.cpp
+3 -1
View File
@@ -183,8 +183,10 @@ void EInkParallelDisplay::asyncFullUpdateTask(void *pvParameters)
self->resetGhostPixelTracking();
#endif
self->asyncFullRunning.store(false);
// Handle first: once asyncFullRunning reads false, the destructor may act on the handle, so
// it must already be null by then (same ordering fix as eink/Drivers/EInkParallel.cpp).
self->asyncTaskHandle = nullptr;
self->asyncFullRunning.store(false);
// delete this task
vTaskDelete(nullptr);
src/graphics/Panel_sdl.cpp
+17 -1
View File
@@ -360,7 +360,10 @@ Panel_sdl::Panel_sdl(void) : Panel_FrameBufferBase()
bool Panel_sdl::init(bool use_reset)
{
initFrameBuffer(_cfg.panel_width * 4, _cfg.panel_height);
// Bail before registering the monitor: continuing with a failed framebuffer allocation
// would leave sdl_update() reading garbage line pointers.
if (!initFrameBuffer(_cfg.panel_width * 4, _cfg.panel_height))
return false;
bool res = Panel_FrameBufferBase::init(use_reset);
_list_monitor.push_back(&monitor);
@@ -647,6 +650,10 @@ bool Panel_sdl::initFrameBuffer(size_t width, size_t height)
}
_texturebuf = (rgb888_t *)heap_alloc_dma(width * height * sizeof(rgb888_t));
if (nullptr == _texturebuf) {
heap_free(lineArray);
return false;
}
/// 8byte alignment;
width = (width + 7) & ~7u;
@@ -655,6 +662,15 @@ bool Panel_sdl::initFrameBuffer(size_t width, size_t height)
memset(lineArray, 0, height * sizeof(uint8_t *));
uint8_t *framebuffer = (uint8_t *)heap_alloc_dma(width * height + 16);
if (nullptr == framebuffer) {
// Returning true here would leave _lines_buffer full of null+offset garbage pointers
// and turn the failure into a wild write on the next redraw.
heap_free(_texturebuf);
_texturebuf = nullptr;
heap_free(lineArray);
_lines_buffer = nullptr;
return false;
}
auto fb = framebuffer;
{
src/graphics/Screen.cpp
+4
View File
@@ -652,6 +652,10 @@ Screen::Screen(ScanI2C::DeviceAddress address, meshtastic_Config_DisplayConfig_O
Screen::~Screen()
{
delete[] graphics::normalFrames;
// Owned by the constructor; Screen is genuinely destroyed on the portduino reboot path
// (screen = nullptr in Power.cpp), which previously leaked the display and UI objects.
delete ui;
delete dispdev;
}
/**
src/graphics/TFTDisplay.cpp
+21 -17
View File
@@ -821,7 +821,7 @@ class LGFX : public lgfx::LGFX_Device
{
lgfx::Bus_SPI _bus_instance;
lgfx::ITouch *_touch_instance;
lgfx::ITouch *_touch_instance = nullptr;
public:
lgfx::Panel_Device *_panel_instance;
@@ -891,24 +891,28 @@ class LGFX : public lgfx::LGFX_Device
} else if (portduino_config.touchscreenModule == ft5x06) {
_touch_instance = new lgfx::Touch_FT5x06;
}
auto touch_cfg = _touch_instance->config();
// Not every module in the config enum has a branch above (gt911 is handled by the
// color-UI path in tftSetup.cpp), so the pointer can legitimately still be null here.
if (_touch_instance) {
auto touch_cfg = _touch_instance->config();
touch_cfg.pin_cs = portduino_config.touchscreenCS.pin;
touch_cfg.x_min = 0;
touch_cfg.x_max = portduino_config.displayHeight - 1;
touch_cfg.y_min = 0;
touch_cfg.y_max = portduino_config.displayWidth - 1;
touch_cfg.pin_int = portduino_config.touchscreenIRQ.pin;
touch_cfg.bus_shared = true;
touch_cfg.offset_rotation = portduino_config.touchscreenRotate;
if (portduino_config.touchscreenI2CAddr != -1) {
touch_cfg.i2c_addr = portduino_config.touchscreenI2CAddr;
} else {
touch_cfg.spi_host = portduino_config.touchscreen_spi_dev_int;
touch_cfg.pin_cs = portduino_config.touchscreenCS.pin;
touch_cfg.x_min = 0;
touch_cfg.x_max = portduino_config.displayHeight - 1;
touch_cfg.y_min = 0;
touch_cfg.y_max = portduino_config.displayWidth - 1;
touch_cfg.pin_int = portduino_config.touchscreenIRQ.pin;
touch_cfg.bus_shared = true;
touch_cfg.offset_rotation = portduino_config.touchscreenRotate;
if (portduino_config.touchscreenI2CAddr != -1) {
touch_cfg.i2c_addr = portduino_config.touchscreenI2CAddr;
} else {
touch_cfg.spi_host = portduino_config.touchscreen_spi_dev_int;
}
_touch_instance->config(touch_cfg);
_panel_instance->setTouch(_touch_instance);
}
_touch_instance->config(touch_cfg);
_panel_instance->setTouch(_touch_instance);
}
#if defined(SDL_h_)
if (portduino_config.displayPanel == x11) {