Fix #4911 : Partially rework some code to remove warnings about potential non-aligned memory accesses (#4912)

* * Adding the -Wcast-align compilation flag for
  the rp2040.

* * Some rework to use a struct to access radio data
* Buffer will not be accessed by arithmetic pointer anymore

* * Remplace arithmetic pointer to avoid Warning

* * Avoid 2 little artitmetic pointer

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>

arch/rp2xx0/rp2040.ini

@@ -7,7 +7,7 @@ platform_packages = framework-arduinopico@https://github.com/earlephilhower/ardu
 board_build.core = earlephilhower
 board_build.filesystem_size = 0.5m
 build_flags = 
-  ${arduino_base.build_flags} -Wno-unused-variable 
+  ${arduino_base.build_flags} -Wno-unused-variable -Wcast-align
   -Isrc/platform/rp2xx0
   -D__PLAT_RP2040__
 #  -D _POSIX_THREADS

src/gps/GPS.cpp

@@ -91,9 +91,9 @@ void GPS::CASChecksum(uint8_t *message, size_t length)
 
     // Iterate over the payload as a series of uint32_t's and
     // accumulate the cksum
-    uint32_t const *payload = (uint32_t *)(message + 6);
     for (size_t i = 0; i < (length - 10) / 4; i++) {
-        uint32_t pl = payload[i];
+        uint32_t pl = 0;
+        memcpy(&pl, (message + 6) + (i * sizeof(uint32_t)), sizeof(uint32_t)); // avoid pointer dereference
         cksum += pl;
     }
 

src/gps/NMEAWPL.cpp

@@ -75,10 +75,13 @@ uint32_t printWPL(char *buf, size_t bufsz, const meshtastic_Position &pos, const
 uint32_t printGGA(char *buf, size_t bufsz, const meshtastic_Position &pos)
 {
     GeoCoord geoCoord(pos.latitude_i, pos.longitude_i, pos.altitude);
-    tm *t = gmtime((time_t *)&pos.timestamp);
+    time_t timestamp = pos.timestamp;
+
+    tm *t = gmtime(&timestamp);
     if (getRTCQuality() > 0) { // use the device clock if we got time from somewhere. If not, use the GPS timestamp.
         uint32_t rtc_sec = getValidTime(RTCQuality::RTCQualityDevice);
-        t = gmtime((time_t *)&rtc_sec);
+        timestamp = rtc_sec;
+        t = gmtime(&timestamp);
     }
 
     uint32_t len = snprintf(

src/mesh/CryptoEngine.cpp

@@ -69,9 +69,8 @@ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, uint64_
     uint32_t *extraNonce;
     long extraNonceTmp = random();
     auth = bytesOut + numBytes;
-    extraNonce = (uint32_t *)(auth + 8);
-    memcpy(extraNonce, &extraNonceTmp,
-           4); // do not use dereference on potential non aligned pointers : *extraNonce = extraNonceTmp;
+    memcpy((uint8_t *)(auth + 8), &extraNonceTmp,
+           sizeof(uint32_t)); // do not use dereference on potential non aligned pointers : *extraNonce = extraNonceTmp;
     LOG_INFO("Random nonce value: %d\n", extraNonceTmp);
     meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(toNode);
     if (node->num < 1 || node->user.public_key.size == 0) {
@@ -88,8 +87,8 @@ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, uint64_
     printBytes("Attempting encrypt using shared_key starting with: ", shared_key, 8);
     aes_ccm_ae(shared_key, 32, nonce, 8, bytes, numBytes, nullptr, 0, bytesOut,
                auth); // this can write up to 15 bytes longer than numbytes past bytesOut
-    memcpy(extraNonce, &extraNonceTmp,
-           4); // do not use dereference on potential non aligned pointers : *extraNonce = extraNonceTmp;
+    memcpy((uint8_t *)(auth + 8), &extraNonceTmp,
+           sizeof(uint32_t)); // do not use dereference on potential non aligned pointers : *extraNonce = extraNonceTmp;
     return true;
 }
 
@@ -105,7 +104,7 @@ bool CryptoEngine::decryptCurve25519(uint32_t fromNode, uint64_t packetNum, size
     uint32_t extraNonce; // pointer was not really used
     auth = bytes + numBytes - 12;
 #ifndef PIO_UNIT_TESTING
-    memcpy(&extraNonce, auth + 8, 4); // do not use dereference on potential non aligned pointers : (uint32_t *)(auth + 8);
+    memcpy(&extraNonce, auth + 8, sizeof(uint32_t)); // do not use dereference on potential non aligned pointers : (uint32_t *)(auth + 8);
     LOG_INFO("Random nonce value: %d\n", extraNonce);
     meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(fromNode);
 

src/mesh/RadioInterface.cpp

@@ -595,26 +595,24 @@ size_t RadioInterface::beginSending(meshtastic_MeshPacket *p)
 
     lastTxStart = millis();
 
-    PacketHeader *h = (PacketHeader *)radiobuf;
-
-    h->from = p->from;
-    h->to = p->to;
-    h->id = p->id;
-    h->channel = p->channel;
-    h->next_hop = 0;   // *** For future use ***
-    h->relay_node = 0; // *** For future use ***
+    radioBuffer.header.from = p->from;
+    radioBuffer.header.to = p->to;
+    radioBuffer.header.id = p->id;
+    radioBuffer.header.channel = p->channel;
+    radioBuffer.header.next_hop = 0;   // *** For future use ***
+    radioBuffer.header.relay_node = 0; // *** For future use ***
     if (p->hop_limit > HOP_MAX) {
         LOG_WARN("hop limit %d is too high, setting to %d\n", p->hop_limit, HOP_RELIABLE);
         p->hop_limit = HOP_RELIABLE;
     }
-    h->flags = p->hop_limit | (p->want_ack ? PACKET_FLAGS_WANT_ACK_MASK : 0) | (p->via_mqtt ? PACKET_FLAGS_VIA_MQTT_MASK : 0);
-    h->flags |= (p->hop_start << PACKET_FLAGS_HOP_START_SHIFT) & PACKET_FLAGS_HOP_START_MASK;
+    radioBuffer.header.flags = p->hop_limit | (p->want_ack ? PACKET_FLAGS_WANT_ACK_MASK : 0) | (p->via_mqtt ? PACKET_FLAGS_VIA_MQTT_MASK : 0);
+    radioBuffer.header.flags |= (p->hop_start << PACKET_FLAGS_HOP_START_SHIFT) & PACKET_FLAGS_HOP_START_MASK;
 
     // if the sender nodenum is zero, that means uninitialized
-    assert(h->from);
+    assert(radioBuffer.header.from);
 
-    memcpy(radiobuf + sizeof(PacketHeader), p->encrypted.bytes, p->encrypted.size);
+    memcpy(radioBuffer.payload, p->encrypted.bytes, p->encrypted.size);
 
     sendingPacket = p;
     return p->encrypted.size + sizeof(PacketHeader);
-}
+}

src/mesh/RadioInterface.h

@@ -45,6 +45,20 @@ typedef struct {
     uint8_t relay_node;
 } PacketHeader;
 
+/**
+ * This structure represent the structured buffer : a PacketHeader then the payload. The whole is
+ * MAX_LORA_PAYLOAD_LEN + 1 length
+ * It makes the use of its data easier, and avoids manipulating pointers (and potential non aligned accesses)
+ */
+typedef struct {
+    /** The header, as defined just before */
+    PacketHeader header;
+
+    /** The payload, of maximum length minus the header, aligned just to be sure */
+    uint8_t payload[MAX_LORA_PAYLOAD_LEN + 1 - sizeof(PacketHeader)] __attribute__ ((__aligned__));
+
+} RadioBuffer;
+
 /**
  * Basic operations all radio chipsets must implement.
  *
@@ -91,8 +105,7 @@ class RadioInterface
     /**
      * A temporary buffer used for sending/receiving packets, sized to hold the biggest buffer we might need
      * */
-    uint8_t radiobuf[MAX_LORA_PAYLOAD_LEN + 1];
-
+    RadioBuffer radioBuffer __attribute__ ((__aligned__));
     /**
      * Enqueue a received packet for the registered receiver
      */

src/mesh/RadioLibInterface.cpp

@@ -387,7 +387,7 @@ void RadioLibInterface::handleReceiveInterrupt()
     }
 #endif
 
-    int state = iface->readData(radiobuf, length);
+    int state = iface->readData((uint8_t*)&radioBuffer, length);
     if (state != RADIOLIB_ERR_NONE) {
         LOG_ERROR("ignoring received packet due to error=%d\n", state);
         rxBad++;
@@ -397,7 +397,6 @@ void RadioLibInterface::handleReceiveInterrupt()
     } else {
         // Skip the 4 headers that are at the beginning of the rxBuf
         int32_t payloadLen = length - sizeof(PacketHeader);
-        const uint8_t *payload = radiobuf + sizeof(PacketHeader);
 
         // check for short packets
         if (payloadLen < 0) {
@@ -405,10 +404,9 @@ void RadioLibInterface::handleReceiveInterrupt()
             rxBad++;
             airTime->logAirtime(RX_ALL_LOG, xmitMsec);
         } else {
-            const PacketHeader *h = (PacketHeader *)radiobuf;
             rxGood++;
             // altered packet with "from == 0" can do Remote Node Administration without permission
-            if (h->from == 0) {
+            if (radioBuffer.header.from == 0) {
                 LOG_WARN("ignoring received packet without sender\n");
                 return;
             }
@@ -418,22 +416,22 @@ void RadioLibInterface::handleReceiveInterrupt()
             // nodes.
             meshtastic_MeshPacket *mp = packetPool.allocZeroed();
 
-            mp->from = h->from;
-            mp->to = h->to;
-            mp->id = h->id;
-            mp->channel = h->channel;
+            mp->from = radioBuffer.header.from;
+            mp->to = radioBuffer.header.to;
+            mp->id = radioBuffer.header.id;
+            mp->channel = radioBuffer.header.channel;
             assert(HOP_MAX <= PACKET_FLAGS_HOP_LIMIT_MASK); // If hopmax changes, carefully check this code
-            mp->hop_limit = h->flags & PACKET_FLAGS_HOP_LIMIT_MASK;
-            mp->hop_start = (h->flags & PACKET_FLAGS_HOP_START_MASK) >> PACKET_FLAGS_HOP_START_SHIFT;
-            mp->want_ack = !!(h->flags & PACKET_FLAGS_WANT_ACK_MASK);
-            mp->via_mqtt = !!(h->flags & PACKET_FLAGS_VIA_MQTT_MASK);
+            mp->hop_limit = radioBuffer.header.flags & PACKET_FLAGS_HOP_LIMIT_MASK;
+            mp->hop_start = (radioBuffer.header.flags & PACKET_FLAGS_HOP_START_MASK) >> PACKET_FLAGS_HOP_START_SHIFT;
+            mp->want_ack = !!(radioBuffer.header.flags & PACKET_FLAGS_WANT_ACK_MASK);
+            mp->via_mqtt = !!(radioBuffer.header.flags & PACKET_FLAGS_VIA_MQTT_MASK);
 
             addReceiveMetadata(mp);
 
             mp->which_payload_variant =
                 meshtastic_MeshPacket_encrypted_tag; // Mark that the payload is still encrypted at this point
             assert(((uint32_t)payloadLen) <= sizeof(mp->encrypted.bytes));
-            memcpy(mp->encrypted.bytes, payload, payloadLen);
+            memcpy(mp->encrypted.bytes, radioBuffer.payload, payloadLen);
             mp->encrypted.size = payloadLen;
 
             printPacket("Lora RX", mp);
@@ -475,7 +473,7 @@ void RadioLibInterface::startSend(meshtastic_MeshPacket *txp)
 
         size_t numbytes = beginSending(txp);
 
-        int res = iface->startTransmit(radiobuf, numbytes);
+        int res = iface->startTransmit((uint8_t*)&radioBuffer, numbytes);
         if (res != RADIOLIB_ERR_NONE) {
             LOG_ERROR("startTransmit failed, error=%d\n", res);
             RECORD_CRITICALERROR(meshtastic_CriticalErrorCode_RADIO_SPI_BUG);