Fix nRF52 remote CLI stack overflows

examples/simple_repeater/MyMesh.cpp

@@ -426,6 +426,20 @@ void MyMesh::sendFloodReply(mesh::Packet* packet, unsigned long delay_millis, ui
   }
 }
 
+void MyMesh::sendClientReplyWithFallbackScope(ClientInfo* client, mesh::Packet* packet,
+                                              unsigned long delay_millis, uint8_t path_hash_size,
+                                              const TransportKey* fallback_scope) {
+  if (packet == NULL) return;
+
+  if (client != NULL && client->out_path_len != OUT_PATH_UNKNOWN) {
+    sendDirect(packet, client->out_path, client->out_path_len, delay_millis);
+  } else if (fallback_scope != NULL) {
+    sendFloodScoped(*fallback_scope, packet, delay_millis, path_hash_size);
+  } else {
+    sendFlood(packet, delay_millis, path_hash_size);
+  }
+}
+
 bool MyMesh::allowPacketForward(const mesh::Packet *packet) {
   if (_prefs.disable_fwd) return false;
   if (packet->isRouteFlood()) {
@@ -715,30 +729,27 @@ void MyMesh::onPeerDataRecv(mesh::Packet *packet, uint8_t type, int sender_idx,
         }
       }
 
-      uint8_t temp[166];
       char *command = (char *)&data[5];
-      char *reply = (char *)&temp[5];
-      if (is_retry) {
-        *reply = 0;
-      } else {
-        handleCommand(sender_timestamp, command, reply);
-      }
-      int text_len = strlen(reply);
-      if (text_len > 0) {
-        uint32_t timestamp = getRTCClock()->getCurrentTimeUnique();
-        if (timestamp == sender_timestamp) {
-          // WORKAROUND: the two timestamps need to be different, in the CLI view
-          timestamp++;
-        }
-        memcpy(temp, &timestamp, 4);        // mostly an extra blob to help make packet_hash unique
-        temp[4] = (TXT_TYPE_CLI_DATA << 2); // NOTE: legacy was: TXT_TYPE_PLAIN
-
-        auto reply = createDatagram(PAYLOAD_TYPE_TXT_MSG, client->id, secret, temp, 5 + text_len);
-        if (reply) {
-          if (client->out_path_len == OUT_PATH_UNKNOWN) {
-            sendFloodReply(reply, CLI_REPLY_DELAY_MILLIS, packet->getPathHashSize());
+      if (!is_retry) {
+        TransportKey reply_scope;
+        const bool reply_scoped =
+            recv_pkt_region != NULL && !recv_pkt_region->isWildcard()
+            && region_map.getTransportKeysFor(*recv_pkt_region, &reply_scope, 1) > 0;
+        size_t command_len = strlen(command);
+        if (!deferred_cli_command.enqueue(i, sender_timestamp, packet->getPathHashSize(),
+                                          secret, command, command_len)) {
+          const char* error = deferred_cli_command.pending
+              ? "Err - another remote command is still running"
+              : "Err - remote command is too long";
+          sendRemoteCliReply(client, secret, packet->getPathHashSize(),
+                             sender_timestamp, error,
+                             reply_scoped ? &reply_scope : NULL);
+        } else {
+          deferred_cli_reply_scoped = reply_scoped;
+          if (reply_scoped) {
+            deferred_cli_reply_scope = reply_scope;
           } else {
-            sendDirect(reply, client->out_path, client->out_path_len, CLI_REPLY_DELAY_MILLIS);
+            memset(deferred_cli_reply_scope.key, 0, sizeof(deferred_cli_reply_scope.key));
           }
         }
       }
@@ -748,6 +759,59 @@ void MyMesh::onPeerDataRecv(mesh::Packet *packet, uint8_t type, int sender_idx,
   }
 }
 
+void MyMesh::sendRemoteCliReply(ClientInfo* client, const uint8_t* secret,
+                                uint8_t path_hash_size, uint32_t sender_timestamp,
+                                const char* reply, const TransportKey* fallback_scope) {
+  if (client == NULL || secret == NULL || reply == NULL || reply[0] == 0) return;
+
+  size_t text_len = strlen(reply);
+  const size_t max_text_len =
+      MAX_PACKET_PAYLOAD - CIPHER_MAC_SIZE - (CIPHER_BLOCK_SIZE - 1) - 5;
+  if (text_len > max_text_len) text_len = max_text_len;
+
+  uint32_t timestamp = getRTCClock()->getCurrentTimeUnique();
+  if (timestamp == sender_timestamp) {
+    // The two timestamps need to differ in the remote CLI view.
+    timestamp++;
+  }
+  memcpy(reply_data, &timestamp, sizeof(timestamp));
+  reply_data[4] = (TXT_TYPE_CLI_DATA << 2);
+  if (reply != (const char*)&reply_data[5]) {
+    memmove(&reply_data[5], reply, text_len);
+  }
+
+  mesh::Packet* packet = createDatagram(PAYLOAD_TYPE_TXT_MSG, client->id, secret,
+                                        reply_data, 5 + text_len);
+  sendClientReplyWithFallbackScope(client, packet, CLI_REPLY_DELAY_MILLIS,
+                                   path_hash_size, fallback_scope);
+}
+
+void __attribute__((noinline)) MyMesh::processDeferredCliCommand() {
+  if (!deferred_cli_command.pending) return;
+
+  const int client_index = deferred_cli_command.client_index;
+  if (client_index < 0 || client_index >= acl.getNumClients()) {
+    MESH_DEBUG_PRINTLN("processDeferredCliCommand: invalid client idx: %d", client_index);
+    deferred_cli_command.clear();
+    deferred_cli_reply_scoped = false;
+    memset(deferred_cli_reply_scope.key, 0, sizeof(deferred_cli_reply_scope.key));
+    return;
+  }
+
+  ClientInfo* client = acl.getClientByIdx(client_index);
+  char* reply = (char*)&reply_data[5];
+  reply[0] = 0;
+  handleCommand(deferred_cli_command.sender_timestamp,
+                deferred_cli_command.command, reply);
+  sendRemoteCliReply(client, deferred_cli_command.secret,
+                     deferred_cli_command.path_hash_size,
+                     deferred_cli_command.sender_timestamp, reply,
+                     deferred_cli_reply_scoped ? &deferred_cli_reply_scope : NULL);
+  deferred_cli_command.clear();
+  deferred_cli_reply_scoped = false;
+  memset(deferred_cli_reply_scope.key, 0, sizeof(deferred_cli_reply_scope.key));
+}
+
 bool MyMesh::onPeerPathRecv(mesh::Packet *packet, int sender_idx, const uint8_t *secret, uint8_t *path,
                             uint8_t path_len, uint8_t extra_type, uint8_t *extra, uint8_t extra_len) {
   // TODO: prevent replay attacks
@@ -861,6 +925,11 @@ MyMesh::MyMesh(mesh::MainBoard &board, mesh::Radio &radio, mesh::MillisecondCloc
   last_millis = 0;
   uptime_millis = 0;
   next_local_advert = next_flood_advert = 0;
+  deferred_cli_reply_scoped = false;
+  memset(deferred_cli_reply_scope.key, 0, sizeof(deferred_cli_reply_scope.key));
+  pending_self_advert_delay = 0;
+  pending_self_advert = false;
+  pending_self_advert_flood = false;
   dirty_contacts_expiry = 0;
   set_radio_at = revert_radio_at = 0;
   _logging = false;
@@ -1012,6 +1081,14 @@ bool MyMesh::formatFileSystem() {
 }
 
 void MyMesh::sendSelfAdvertisement(int delay_millis, bool flood) {
+  // CommonCLI can invoke this while handling an encrypted remote-admin packet.
+  // Defer Ed25519 signing until the command and receive call stacks unwind.
+  pending_self_advert_delay = delay_millis > 0 ? (uint32_t)delay_millis : 0;
+  pending_self_advert_flood = flood;
+  pending_self_advert = true;
+}
+
+void MyMesh::sendSelfAdvertisementNow(uint32_t delay_millis, bool flood) {
   mesh::Packet *pkt = createSelfAdvert();
   if (pkt) {
     if (flood) {
@@ -1267,6 +1344,17 @@ void MyMesh::loop() {
 #endif
 
   mesh::Mesh::loop();
+  processDeferredCliCommand();
+  servicePostMeshLoop();
+}
+
+void __attribute__((noinline)) MyMesh::servicePostMeshLoop() {
+  if (pending_self_advert) {
+    const uint32_t delay_millis = pending_self_advert_delay;
+    const bool flood = pending_self_advert_flood;
+    pending_self_advert = false;
+    sendSelfAdvertisementNow(delay_millis, flood);
+  }
 
   if (next_flood_advert && millisHasNowPassed(next_flood_advert)) {
     mesh::Packet *pkt = createSelfAdvert();
@@ -1308,6 +1396,7 @@ void MyMesh::loop() {
 
 // To check if there is pending work
 bool MyMesh::hasPendingWork() const {
+  if (deferred_cli_command.pending || pending_self_advert) return true;
 #if defined(WITH_BRIDGE)
   if (bridge.isRunning()) return true;  // bridge needs WiFi radio, can't sleep
 #endif

examples/simple_repeater/MyMesh.h

@@ -28,6 +28,7 @@
 #include <helpers/ArduinoHelpers.h>
 #include <helpers/ClientACL.h>
 #include <helpers/CommonCLI.h>
+#include <helpers/DeferredCliCommand.h>
 #include <helpers/IdentityStore.h>
 #include <helpers/SimpleMeshTables.h>
 #include <helpers/StaticPoolPacketManager.h>
@@ -86,6 +87,12 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks {
   uint32_t last_millis;
   uint64_t uptime_millis;
   unsigned long next_local_advert, next_flood_advert;
+  mesh::DeferredCliCommand deferred_cli_command;
+  TransportKey deferred_cli_reply_scope;
+  bool deferred_cli_reply_scoped;
+  uint32_t pending_self_advert_delay;
+  bool pending_self_advert;
+  bool pending_self_advert_flood;
   bool _logging;
   NodePrefs _prefs;
   ClientACL  acl;
@@ -127,6 +134,15 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks {
   uint8_t handleAnonClockReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data);
   int handleRequest(ClientInfo* sender, uint32_t sender_timestamp, uint8_t* payload, size_t payload_len);
   mesh::Packet* createSelfAdvert();
+  void processDeferredCliCommand();
+  void sendRemoteCliReply(ClientInfo* client, const uint8_t* secret,
+                          uint8_t path_hash_size, uint32_t sender_timestamp,
+                          const char* reply, const TransportKey* fallback_scope);
+  void sendClientReplyWithFallbackScope(ClientInfo* client, mesh::Packet* packet,
+                                        unsigned long delay_millis, uint8_t path_hash_size,
+                                        const TransportKey* fallback_scope);
+  void servicePostMeshLoop();
+  void sendSelfAdvertisementNow(uint32_t delay_millis, bool flood);
 
   File openAppend(const char* fname);
   bool isLooped(const mesh::Packet* packet, const uint8_t max_counters[]);

examples/simple_repeater/main.cpp

@@ -122,7 +122,7 @@ void setup() {
   board.onBootComplete();
 }
 
-void loop() {
+static void __attribute__((noinline)) serviceCommandInterfaces() {
   // Handle Serial CLI
   int len = strlen(command);
   while (Serial.available() && len < sizeof(command)-1) {
@@ -169,6 +169,10 @@ void loop() {
     ethernet_command[0] = 0;
   }
 #endif
+}
+
+void loop() {
+  serviceCommandInterfaces();
 
 #if defined(PIN_USER_BTN) && defined(_SEEED_SENSECAP_SOLAR_H_) && !defined(DISPLAY_CLASS)
   // Hold the user button to power off the SenseCAP Solar repeater.

platformio.ini

@@ -92,6 +92,8 @@ build_flags = ${arduino_base.build_flags}
   -D LFS_NO_ASSERT=1
   -D EXTRAFS=1
   -D USE_CC310_HW_CRYPTO=1
+  -D MESH_NRF52_LOOP_STACK_WORDS=2048
+  -Wl,--wrap=xTaskCreate
 lib_deps =
   ${arduino_base.lib_deps}
   https://github.com/oltaco/CustomLFS#0.2.2

src/Nrf52LoopStack.cpp

@@ -0,0 +1,39 @@
+#if defined(NRF52_PLATFORM)
+
+#include <Arduino.h>
+#include <FreeRTOS.h>
+#include <task.h>
+#include <string.h>
+
+#ifndef MESH_NRF52_LOOP_STACK_WORDS
+  #define MESH_NRF52_LOOP_STACK_WORDS 2048
+#endif
+
+static_assert(MESH_NRF52_LOOP_STACK_WORDS >= 1024,
+              "The nRF52 loop task stack must not be smaller than the framework default");
+
+extern "C" BaseType_t __real_xTaskCreate(
+    TaskFunction_t task_code, const char* const task_name,
+    const configSTACK_DEPTH_TYPE stack_depth, void* const parameters,
+    UBaseType_t priority, TaskHandle_t* const created_task);
+
+// The Adafruit nRF52 core hard-codes the Arduino loop task to 1024 32-bit
+// words (4 KiB). Mesh packet verification and authenticated remote commands
+// can legitimately enter Ed25519 routines whose stack peaks do not fit there.
+// Wrap task creation so only the framework's "loop" task receives 8 KiB; all
+// BLE, timer, callback, and application-created tasks retain their requested
+// sizes.
+extern "C" BaseType_t __wrap_xTaskCreate(
+    TaskFunction_t task_code, const char* const task_name,
+    const configSTACK_DEPTH_TYPE stack_depth, void* const parameters,
+    UBaseType_t priority, TaskHandle_t* const created_task) {
+  configSTACK_DEPTH_TYPE adjusted_depth = stack_depth;
+  if (task_name != NULL && strcmp(task_name, "loop") == 0
+      && stack_depth == 1024) {
+    adjusted_depth = MESH_NRF52_LOOP_STACK_WORDS;
+  }
+  return __real_xTaskCreate(task_code, task_name, adjusted_depth, parameters,
+                            priority, created_task);
+}
+
+#endif

src/helpers/DeferredCliCommand.h

@@ -0,0 +1,57 @@
+#pragma once
+
+#include <MeshCore.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <string.h>
+
+namespace mesh {
+
+// A single-entry mailbox for authenticated remote CLI commands. The repeater
+// drains it immediately after Mesh::loop() returns; a second command arriving
+// in the same pass receives a busy response. Keeping the command in object
+// storage lets the receive/decrypt call chain unwind before command handlers
+// perform filesystem or cryptographic work.
+struct DeferredCliCommand {
+  bool pending;
+  int client_index;
+  uint32_t sender_timestamp;
+  uint8_t path_hash_size;
+  uint8_t secret[PUB_KEY_SIZE];
+  char command[MAX_PACKET_PAYLOAD + 1];
+
+  DeferredCliCommand()
+      : pending(false), client_index(-1), sender_timestamp(0), path_hash_size(1) {
+    memset(secret, 0, sizeof(secret));
+    command[0] = 0;
+  }
+
+  bool enqueue(int new_client_index, uint32_t new_sender_timestamp,
+               uint8_t new_path_hash_size, const uint8_t* new_secret,
+               const char* new_command, size_t command_len) {
+    if (pending || new_secret == NULL || new_command == NULL
+        || command_len >= sizeof(command)) {
+      return false;
+    }
+
+    client_index = new_client_index;
+    sender_timestamp = new_sender_timestamp;
+    path_hash_size = new_path_hash_size;
+    memcpy(secret, new_secret, sizeof(secret));
+    memcpy(command, new_command, command_len);
+    command[command_len] = 0;
+    pending = true;
+    return true;
+  }
+
+  void clear() {
+    pending = false;
+    client_index = -1;
+    sender_timestamp = 0;
+    path_hash_size = 1;
+    memset(secret, 0, sizeof(secret));
+    memset(command, 0, sizeof(command));
+  }
+};
+
+} // namespace mesh