From 66971a0a267f29047fc32a6197841e3cf37431c8 Mon Sep 17 00:00:00 2001
From: nightjoker7 <47129685+nightjoker7@users.noreply.github.com>
Date: Thu, 23 Apr 2026 06:16:08 -0500
Subject: [PATCH] RadioLibInterface: clear static `instance` on destruction to
 prevent UAF (#10254)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The constructor sets `RadioLibInterface::instance = this` immediately,
before `init()` runs. `initLoRa()` in RadioInterface.cpp creates each
radio variant with `new SX1262Interface(...)` or similar, then calls
`init()`, and if init fails the `unique_ptr<RadioInterface>` is reset
to nullptr — destroying the object — while the static `instance`
pointer continues to point at the freed memory.

Main loop then checks `RadioLibInterface::instance != nullptr` and
calls `pollMissedIrqs()` or `resetAGC()` on the dangling pointer →
Guru Meditation (IllegalInstruction / LoadProhibited).

Reported in #9880 on an ESP32-S3 dev board without radio hardware
attached, where init always fails and the leftover pointer crashes
the device on the next `loop()` iteration.

Fix: add a virtual destructor to `RadioLibInterface` that clears the
static pointer iff it still references this object. A later successful
init() may have replaced `instance` with a different interface — the
`instance == this` guard preserves that case.

Fixes #9880

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
---
 src/mesh/RadioLibInterface.cpp | 10 ++++++++++
 src/mesh/RadioLibInterface.h   |  7 +++++++
 2 files changed, 17 insertions(+)

diff --git a/src/mesh/RadioLibInterface.cpp b/src/mesh/RadioLibInterface.cpp
index 7ef707e0d..6024d06b6 100644
--- a/src/mesh/RadioLibInterface.cpp
+++ b/src/mesh/RadioLibInterface.cpp
@@ -46,6 +46,16 @@ RadioLibInterface::RadioLibInterface(LockingArduinoHal *hal, RADIOLIB_PIN_TYPE c
 #endif
 }
 
+RadioLibInterface::~RadioLibInterface()
+{
+    // If the static `instance` pointer still references us, clear it.
+    // A later successful init() may have replaced `instance` with a newer
+    // interface — don't clobber that case.
+    if (instance == this) {
+        instance = nullptr;
+    }
+}
+
 #ifdef ARCH_ESP32
 // ESP32 doesn't use that flag
 #define YIELD_FROM_ISR(x) portYIELD_FROM_ISR()
diff --git a/src/mesh/RadioLibInterface.h b/src/mesh/RadioLibInterface.h
index 310ca76bb..2859558ed 100644
--- a/src/mesh/RadioLibInterface.h
+++ b/src/mesh/RadioLibInterface.h
@@ -136,6 +136,13 @@ class RadioLibInterface : public RadioInterface, protected concurrency::Notified
     RadioLibInterface(LockingArduinoHal *hal, RADIOLIB_PIN_TYPE cs, RADIOLIB_PIN_TYPE irq, RADIOLIB_PIN_TYPE rst,
                       RADIOLIB_PIN_TYPE busy, PhysicalLayer *iface = NULL);
 
+    /**
+     * Clear the static `instance` pointer if it still points at us, so callers
+     * that check `RadioLibInterface::instance != nullptr` don't dereference a
+     * freed object after a failed init() + unique_ptr reset.
+     */
+    virtual ~RadioLibInterface();
+
     virtual ErrorCode send(meshtastic_MeshPacket *p) override;
 
     /**