fix(mesh): harden MemoryPool<T,MaxSize>::release() against bad pointers - #11232
fix(mesh): harden MemoryPool<T,MaxSize>::release() against bad pointers#11232ndoo wants to merge 1 commit into
Conversation
int index = p - pool is UB unless p already points into pool, and a double-free hit assert(used[index]) - a hang with no diagnostic on STM32WL, where clientNotificationPool/staticQueueStatusPool use this static pool. Validate via address arithmetic instead of raw pointer subtraction, and log-and-return on double-free instead of asserting, matching the MemoryDynamic::alloc() precedent (meshtastic#11197). Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⚡ Try this PR in the Web FlasherNote Building this pull request… the flash button, badges and supported-board |
Problem
MemoryPool<T,MaxSize>::release()(src/mesh/MemoryPool.h) computedint index = p - pool;— pointer subtraction that's undefined behavior unlesspalready points intopool. A double-free hitassert(used[index]), which hangs forever with no diagnostic on STM32WL (__wrap___assert_funciswhile(true);).This static pool backs
clientNotificationPool/staticQueueStatusPool/staticMqttClientProxyMessagePoolon every platform including STM32WL (unlikepacketPool, which isMemoryDynamic-backed there) — so this is reachable, not theoretical.Fix
Validate via address arithmetic (
reinterpret_cast<uintptr_t>) instead of raw pointer subtraction, rejecting a foreign/misaligned pointer before it's used as an index. Convert the double-freeassert()into a logged, graceful return — matching theMemoryDynamic::alloc()precedent from #11197.Test plan
pio run -e wio-e5/pio run -e rak3172— build clean.meshtastic_ClientNotification, releases it, then deliberately releases the same pointer again and a misaligned pointer. Confirmed over serial:"Double release of pool item 0 at 0x..." and"Pointer 0x... not from our pool!"logged, no hang, boot continues normally (radio init succeeds). Test hook reverted before opening this PR.🤝 Attestations