Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanitizer: heap use after delete #3373

Merged
merged 2 commits into from
Mar 11, 2024
Merged

Sanitizer: heap use after delete #3373

merged 2 commits into from
Mar 11, 2024

Conversation

mverch67
Copy link
Collaborator

@mverch67 mverch67 commented Mar 11, 2024

On native when using the gcc sanitizer the program stops and aborts with a stacktrace when sending a message.

I moved the code that accesses the deleted heap space just before the deletion. Tested with target native and t3s3.

`DEBUG | 14:12:37 2512 [Router] Rx someone rebroadcasting for us (id=0x327b2443 fr=0x7c to=0x7c, WantAck=0, HopLim=3 Ch=0x0 Portnum=5 requestId=327b2442 rxtime=1710166357 priority=120)
DEBUG | 14:12:37 2512 [Router] didn't find pending packet
DEBUG | 14:12:37 2512 [Router] (bw=250, sf=11, cr=4/5) packet symLen=8 ms, payloadSize=27, time 436 ms
DEBUG | 14:12:37 2512 [Router] recentPackets size=609
DEBUG | 14:12:37 2512 [Router] recentPackets size=609 (after clearing expired packets)
DEBUG | 14:12:37 2512 [Router] Add packet record (id=0x327b2443 fr=0x7c to=0x7c, WantAck=0, HopLim=3 Ch=0x0 Portnum=5 requestId=327b2442 rxtime=1710166357 priority=120)
DEBUG | 14:12:37 2512 [Router] recentPackets size=610
DEBUG | 14:12:37 2512 [Router] recentPackets size=610 (after clearing expired packets)
DEBUG | 14:12:37 2512 [Router] handleReceived(REMOTE) (id=0x327b2443 fr=0x7c to=0x7c, WantAck=0, HopLim=3 Ch=0x0 Portnum=5 requestId=327b2442 rxtime=1710166357 priority=120)
DEBUG | 14:12:37 2512 [Router] Module 'canned' wantsPacket=1
DEBUG | 14:12:37 2512 [Router] Module 'canned' considered
DEBUG | 14:12:37 2512 [Router] Module 'routing' wantsPacket=1
INFO | 14:12:37 2512 [Router] Received routing from=0xe8f1687c, id=0x327b2443, portnum=5, payloadlen=2
DEBUG | 14:12:37 2512 [Router] Routing sniffing (id=0x327b2443 fr=0x7c to=0x7c, WantAck=0, HopLim=3 Ch=0x0 Portnum=5 requestId=327b2442 rxtime=1710166357 priority=120)
DEBUG | 14:12:37 2512 [Router] Received an ack for 0x327b2442, stopping retransmissions

==664740==
ERROR: AddressSanitizer: heap-use-after-free on address 0x6040000a29ac at pc 0x00000050ab8a bp 0x7fffffffc380 sp 0x7fffffffc378
READ of size 1 at 0x6040000a29ac thread T0
#0 0x50ab89 in ReliableRouter::stopRetransmission(GlobalPacketId) src/mesh/ReliableRouter.cpp:174
#1 0x50a9c6 in ReliableRouter::stopRetransmission(unsigned int, unsigned int) src/mesh/ReliableRouter.cpp:162
#2 0x50a5a4 in ReliableRouter::sniffReceived(_meshtastic_MeshPacket const*, _meshtastic_Routing const*) src/mesh/ReliableRouter.cpp:128
#3 0x552d75 in RoutingModule::handleReceivedProtobuf(_meshtastic_MeshPacket const&, _meshtastic_Routing*) src/modules/RoutingModule.cpp:13
#4 0x553a4b in ProtobufModule<_meshtastic_Routing>::handleReceived(_meshtastic_MeshPacket const&) src/mesh/ProtobufModule.h:91
#5 0x4d2be6 in MeshModule::callPlugins(_meshtastic_MeshPacket&, RxSource) src/mesh/MeshModule.cpp:128
#6 0x51342f in Router::handleReceived(_meshtastic_MeshPacket*, RxSource) src/mesh/Router.cpp:480
#7 0x513770 in Router::perhapsHandleReceived(_meshtastic_MeshPacket*) src/mesh/Router.cpp:497
#8 0x50fa4c in Router::runOnce() src/mesh/Router.cpp:71
#9 0x50c199 in ReliableRouter::runOnce() src/mesh/ReliableRouter.h:79
#10 0x458808 in concurrency::OSThread::run() src/concurrency/OSThread.cpp:85
#11 0x6086b6 in ThreadController::runOrDelay() .pio/libdeps/native-x11-320x240/Thread/ThreadController.cpp:59
#12 0x4bbfab in loop src/main.cpp:981
#13 0x7872b8 in main /home/manuel/.platformio/packages/framework-portduino/cores/portduino/main.cpp:209
#14 0x7ffff7165149 in __libc_start_call_main (/usr/lib64/libc.so.6+0x28149) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756)
#15 0x7ffff716520a in __libc_start_main_impl (/usr/lib64/libc.so.6+0x2820a) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756)
#16 0x40a304 in _start (/home/manuel/Documents/PlatformIO/Projects/meshtastic-firmware/.pio/build/native-x11-320x240/program+0x40a304) (BuildId: ddb59d8371c4c5197fcc8bbbac4c10b6dccb206d)

0x6040000a29ac is located 28 bytes inside of 40-byte region [0x6040000a2990,0x6040000a29b8)
freed by thread T0 here:
#0 0x7ffff78dad28 in operator delete(void*, unsigned long) (/usr/lib64/libasan.so.8+0xdad28) (BuildId: 7fcb7759bc17ef47f9682414b6d99732d6a6ab0c)
#1 0x50ee89 in std::__new_allocator<std::__detail::_Hash_node<std::pair<GlobalPacketId const, PendingPacket>, true> >::deallocate(std::__detail::_Hash_node<std::pair<GlobalPacketId const, PendingPacket>, true>, unsigned long) /usr/include/c++/13/bits/new_allocator.h:172
#2 0x50ea0e in std::allocator_traits<std::allocator<std::__detail::_Hash_node<std::pair<GlobalPacketId const, PendingPacket>, true> > >::deallocate(std::allocator<std::__detail::_Hash_node<std::pair<GlobalPacketId const, PendingPacket>, true> >&, std::__detail::_Hash_node<std::pair<GlobalPacketId const, PendingPacket>, true>
, unsigned long) /usr/include/c++/13/bits/alloc_traits.h:517
#3 0x50ea0e in std::__detail::_Hashtable_alloc<std::allocator<std::__detail::_Hash_node<std::pair<GlobalPacketId const, PendingPacket>, true> > >::_M_deallocate_node_ptr(std::__detail::_Hash_node<std::pair<GlobalPacketId const, PendingPacket>, true>) /usr/include/c++/13/bits/hashtable_policy.h:2022
#4 0x50e28d in std::__detail::_Hashtable_alloc<std::allocator<std::__detail::_Hash_node<std::pair<GlobalPacketId const, PendingPacket>, true> > >::_M_deallocate_node(std::__detail::_Hash_node<std::pair<GlobalPacketId const, PendingPacket>, true>
) /usr/include/c++/13/bits/hashtable_policy.h:2012
#5 0x50dda5 in std::_Hashtable<GlobalPacketId, std::pair<GlobalPacketId const, PendingPacket>, std::allocator<std::pair<GlobalPacketId const, PendingPacket> >, std::__detail::_Select1st, std::equal_to, GlobalPacketIdHashFunction, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::_M_erase(unsigned long, std::__detail::_Hash_node_base*, std::__detail::_Hash_node<std::pair<GlobalPacketId const, PendingPacket>, true>) /usr/include/c++/13/bits/hashtable.h:2353
#6 0x50d22a in std::_Hashtable<GlobalPacketId, std::pair<GlobalPacketId const, PendingPacket>, std::allocator<std::pair<GlobalPacketId const, PendingPacket> >, std::__detail::_Select1st, std::equal_to, GlobalPacketIdHashFunction, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::_M_erase(std::integral_constant<bool, true>, GlobalPacketId const&) /usr/include/c++/13/bits/hashtable.h:2396
#7 0x50cada in std::_Hashtable<GlobalPacketId, std::pair<GlobalPacketId const, PendingPacket>, std::allocator<std::pair<GlobalPacketId const, PendingPacket> >, std::__detail::_Select1st, std::equal_to, GlobalPacketIdHashFunction, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::erase(GlobalPacketId const&) /usr/include/c++/13/bits/hashtable.h:984
#8 0x50c4ee in std::unordered_map<GlobalPacketId, PendingPacket, GlobalPacketIdHashFunction, std::equal_to, std::allocator<std::pair<GlobalPacketId const, PendingPacket> > >::erase(GlobalPacketId const&) /usr/include/c++/13/bits/unordered_map.h:770
#9 0x50ab16 in ReliableRouter::stopRetransmission(GlobalPacketId) src/mesh/ReliableRouter.cpp:170
#10 0x50a9c6 in ReliableRouter::stopRetransmission(unsigned int, unsigned int) src/mesh/ReliableRouter.cpp:162
#11 0x50a5a4 in ReliableRouter::sniffReceived(_meshtastic_MeshPacket const
, _meshtastic_Routing const*) src/mesh/ReliableRouter.cpp:128
#12 0x552d75 in RoutingModule::handleReceivedProtobuf(_meshtastic_MeshPacket const&, _meshtastic_Routing*) src/modules/RoutingModule.cpp:13
#13 0x553a4b in ProtobufModule<_meshtastic_Routing>::handleReceived(_meshtastic_MeshPacket const&) src/mesh/ProtobufModule.h:91
#14 0x4d2be6 in MeshModule::callPlugins(_meshtastic_MeshPacket&, RxSource) src/mesh/MeshModule.cpp:128
#15 0x51342f in Router::handleReceived(_meshtastic_MeshPacket*, RxSource) src/mesh/Router.cpp:480
#16 0x513770 in Router::perhapsHandleReceived(_meshtastic_MeshPacket*) src/mesh/Router.cpp:497
#17 0x50fa4c in Router::runOnce() src/mesh/Router.cpp:71
#18 0x50c199 in ReliableRouter::runOnce() src/mesh/ReliableRouter.h:79
#19 0x458808 in concurrency::OSThread::run() src/concurrency/OSThread.cpp:85
#20 0x6086b6 in ThreadController::runOrDelay() .pio/libdeps/native-x11-320x240/Thread/ThreadController.cpp:59
#21 0x4bbfab in loop src/main.cpp:981
#22 0x7872b8 in main /home/manuel/.platformio/packages/framework-portduino/cores/portduino/main.cpp:209`

@thebentern thebentern merged commit e16689a into master Mar 11, 2024
70 checks passed
upchui added a commit to upchui/firmware-Meshtastic-Unleashed that referenced this pull request Mar 11, 2024
* Move Wireless Paper V1.1 custom hibernate behavior to GxEPD2

* Async full-refresh for EInkDynamicDisplay

* initial config for T-Echo

* formatting
responds to meshtastic#3339 (comment)

* increase fast-refresh limit for T-Echo
meshtastic#3339 (comment)

* change dependency from private repo to meshtastic/GxEPD2

* rename setFrameFlag() method

* move storeAndReset() to end of update()

* change order of determineMode() checks

* add init code as a determineMode() check

* add BLOCKING modifier to frameFlagTypes

* add frameFlags to LOG_DEBUG() messages

* function macro for tidier addFramFlag() calls

* handle special frames in Screen.cpp

* fix fallback behavior for unmodified GxEPD2
Issues exposed by meshtastic#3356 (comment)

* while drafting, build from todd-herbert/meshtastic-GxEPD2#async

* reorder determineMode() checks

* Swap Wireless Paper V1.0 dependency to meshtastic/GxEPD2

* purge unused enum val

* refactor: remove ACKs in range tests so zero hops is honored (meshtastic#3374)

* fix heap use after delete (meshtastic#3373)

* fix of tryfix SHT31 sensor (meshtastic#3377)

* Don't spam logs if no position with map reporting (meshtastic#3378)

---------

Co-authored-by: Todd Herbert <herbert.todd@gmail.com>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>
Co-authored-by: Andre K <andrekir@pm.me>
Co-authored-by: Manuel <71137295+mverch67@users.noreply.github.com>
Co-authored-by: GUVWAF <78759985+GUVWAF@users.noreply.github.com>
@caveman99 caveman99 deleted the sanitizer branch April 9, 2024 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants