meshtastic / firmware

Meshtastic device firmware
https://meshtastic.org
GNU General Public License v3.0
2.97k stars 711 forks source link

fix crash during reset nodedb #4017

Closed mverch67 closed 3 weeks ago

mverch67 commented 3 weeks ago

This PR fixes a crash during reset NodeDB (dereferencing null pointer).

The order of statements in resetNodes() is wrong. First, numMeshNodes is set to 1, but after that (in clearLocalPosition()) the nodeId is searched in the meshDB, which cannot be found anymore, because meshDB is set to one element already.

Note: normally this may not happen, as the own node is at position 0 (the only one element which is found). But when running MQTT with hundreds of nodes the own node seems not at position 0 anymore -> crash.

Another question is: who wrote code that reorders the own node away from position 0? Should not be necessary at all, right?

mverch67 commented 3 weeks ago

After some thinking I have an idea what the root cause is that the own node is not at position 0. After receiving the admin message and purging meshDB there are upto 7s until the node reboots. If new nodes are advertised (as in case of MQTT happens every second) they are inserted into the db. So after the reboot these new nodes are already in the nodeDB. Also, if for some reason the own nodeId is regenerated (e.g. using parameter -h on native) the own node will not occupy position 0 anymore.

Another problem I saw is now the own nodeId will never be at position 0 again, because the line

std::fill(devicestate.node_db_lite.begin() + 1, devicestate.node_db_lite.end(), meshtastic_NodeInfoLite());

is not removing the actual node at position 0 !! So at next reboot our own node will be at position 1 in the meshDB.

Anyway, the fix in this PR prevents the crash as a result of the moving of the own node away from position 0.