Various codeQL fixes

Code was previously performing arithmetic in
various loop check conditions.  Integer promotion rules could cause unintended comparisons.

`spiffs` defined `fs->block_count` as `uint32_t`, but defined `spiffs_page_ix` as `uint16_t`.  Various overflow checks detected by CodeQL and fixed by checking for those conditions before looping.

armsrc/spiffs_check.c

@@ -535,33 +535,61 @@ static s32_t spiffs_page_consistency_check_i(spiffs *fs) {
 
     s32_t res = SPIFFS_OK;
     spiffs_page_ix pix_offset = 0;
+    // Avoid arithmetic in loop conditions (integer promotion rules can cause unintended consequences)
+    uint32_t block_count = fs->block_count;
+    uint32_t total_blocks = SPIFFS_PAGES_PER_BLOCK(fs) * block_count;
+    uint32_t total_blocks_plus_one_page = total_blocks + SPIFFS_PAGES_PER_BLOCK(fs);
 
+#pragma region    // check for overflow once, before looping
     // this _should_ never happen, but prefer to see debug message / error
     // rather than silently entering infinite loop.
-    if (fs->block_count > ((spiffs_block_ix)(-1))) {
-        SPIFFS_DBG("Avoiding infinite loop, block_count "_SPIPRIbl" too large for spiffs_block_ix type\n", fs->block_count);
+    if (block_count > ((spiffs_block_ix)(-1))) {
+        SPIFFS_DBG("Avoiding infinite loop, block_count "_SPIPRIbl" too large for spiffs_block_ix type\n", block_count);
         SPIFFS_API_CHECK_RES(fs, SPIFFS_ERR_INTERNAL);
     }
+    // this checks for overflow of the multiplication of block_count+1 with SPIFFS_PAGES_PER_BLOCK(fs)
+    if (((uint32_t)(-1)) / SPIFFS_PAGES_PER_BLOCK(fs) > (block_count+1)) {
+        // checking with +1 block count to avoid overflow also in inner loop, which adds one page...
+        // would exceed value storable in uint32_t
+        SPIFFS_DBG("Overflow: pages per block %04x with block count "_SPIPRIbl" results in overflow\n", SPIFFS_PAGES_PER_BLOCK(fs), block_count);
+        SPIFFS_API_CHECK_RES(fs, SPIFFS_ERR_INTERNAL);
+    }
+    // because loop indices are using spiffs_page_ix type,
+    // that type can hold a large enough value
+    if (total_blocks > ((spiffs_page_ix)-1)) {
+        SPIFFS_DBG("Avoiding infinite loop, total_blocks "_SPIPRIpg" too large for spiffs_page_ix type\n", total_blocks);
+        SPIFFS_CHECK_RES(SPIFFS_ERR_INTERNAL);
+    }
+    // because loop indices are using spiffs_page_ix type,
+    // that type can hold a large enough value
+    if (total_blocks_plus_one_page > ((spiffs_page_ix)-1) || total_blocks_plus_one_page < total_blocks) {
+        SPIFFS_DBG("Avoiding infinite loop, total_blocks_plus_one_page "_SPIPRIpg" too large for spiffs_page_ix type\n", total_blocks_plus_one_page);
+        SPIFFS_CHECK_RES(SPIFFS_ERR_INTERNAL);
+    }
+    // RESULT: spiffs_page_ix can safely be used for loop index vs. each of
+    //         block_count, total_blocks, and total_blocks_plus_one_page
+#pragma endregion // check for overflow once, before looping
+
 
     // for each range of pages fitting into work memory
-    while (pix_offset < SPIFFS_PAGES_PER_BLOCK(fs) * fs->block_count) {
+    while (pix_offset < total_blocks) {
         // set this flag to abort all checks and rescan the page range
         u8_t restart = 0;
         memset(fs->work, 0, SPIFFS_CFG_LOG_PAGE_SZ(fs));
 
         spiffs_block_ix cur_block = 0;
         // build consistency bitmap for id range traversing all blocks
-        while (!restart && cur_block < fs->block_count) {
+        while (!restart && cur_block < block_count) {
             CHECK_CB(fs, SPIFFS_CHECK_PAGE, SPIFFS_CHECK_PROGRESS,
-                     (pix_offset * 256) / (SPIFFS_PAGES_PER_BLOCK(fs) * fs->block_count) +
-                     ((((cur_block * pages_per_scan * 256) / (SPIFFS_PAGES_PER_BLOCK(fs) * fs->block_count))) / fs->block_count),
+                     (pix_offset * 256) / total_blocks +
+                     ((((cur_block * pages_per_scan * 256) / total_blocks)) / block_count),
                      0);
             // traverse each page except for lookup pages
             spiffs_page_ix cur_pix = SPIFFS_OBJ_LOOKUP_PAGES(fs) + SPIFFS_PAGES_PER_BLOCK(fs) * cur_block;
-            while (!restart && cur_pix < SPIFFS_PAGES_PER_BLOCK(fs) * (cur_block + 1)) {
+            while (!restart && cur_pix < total_blocks_plus_one_page) {
                 //if ((cur_pix & 0xff) == 0)
                 //  SPIFFS_CHECK_DBG("PA: processing pix "_SPIPRIpg", block "_SPIPRIbl" of pix "_SPIPRIpg", block "_SPIPRIbl"\n",
-                //      cur_pix, cur_block, SPIFFS_PAGES_PER_BLOCK(fs) * fs->block_count, fs->block_count);
+                //      cur_pix, cur_block, total_blocks, block_count);
 
                 // read header
                 spiffs_page_header p_hdr;

armsrc/spiffs_hydrogen.c

@@ -52,15 +52,16 @@ s32_t SPIFFS_format(spiffs *fs) {
 
     SPIFFS_LOCK(fs);
 
+    uint32_t block_count = fs->block_count;
     // this _should_ never happen, but prefer to see debug message / error
     // rather than silently entering infinite loop.
-    if (fs->block_count > ((spiffs_block_ix)(-1))) {
-        SPIFFS_DBG("Avoiding infinite loop, block_count "_SPIPRIbl" too large for spiffs_block_ix type\n", fs->block_count);
+    if (block_count > ((spiffs_block_ix)(-1))) {
+        SPIFFS_DBG("Avoiding infinite loop, block_count "_SPIPRIbl" too large for spiffs_block_ix type\n", block_count);
         SPIFFS_API_CHECK_RES_UNLOCK(fs, SPIFFS_ERR_INTERNAL);
     }
 
     spiffs_block_ix bix = 0;
-    while (bix < fs->block_count) {
+    while (bix < block_count) {
         fs->max_erase_count = 0;
         s32_t res = spiffs_erase_block(fs, bix);
         if (res != SPIFFS_OK) {

armsrc/spiffs_nucleus.c

@@ -372,10 +372,11 @@ s32_t spiffs_obj_lu_scan(
     spiffs_block_ix unerased_bix = (spiffs_block_ix) - 1;
 #endif
 
+    uint32_t block_count = fs->block_count;
     // this _should_ never happen, but prefer to see debug message / error
     // rather than silently entering infinite loop.
-    if (fs->block_count > ((spiffs_block_ix)(-1))) {
-        SPIFFS_DBG("Avoiding infinite loop, block_count "_SPIPRIbl" too large for spiffs_block_ix type\n", fs->block_count);
+    if (block_count > ((spiffs_block_ix)(-1))) {
+        SPIFFS_DBG("Avoiding infinite loop, block_count "_SPIPRIbl" too large for spiffs_block_ix type\n", block_count);
         SPIFFS_API_CHECK_RES(fs, SPIFFS_ERR_INTERNAL);
     }
 
@@ -386,7 +387,7 @@ s32_t spiffs_obj_lu_scan(
     spiffs_obj_id erase_count_final;
     spiffs_obj_id erase_count_min = SPIFFS_OBJ_ID_FREE;
     spiffs_obj_id erase_count_max = 0;
-    while (bix < fs->block_count) {
+    while (bix < block_count) {
 #if SPIFFS_USE_MAGIC
         spiffs_obj_id magic;
         res = _spiffs_rd(fs,

armsrc/thinfilm.c

@@ -105,7 +105,7 @@ static int EmSendCmdThinfilmRaw(const uint8_t *resp, uint16_t respLen) {
     uint16_t FpgaSendQueueDelay = 0;
 
     // send cycle
-    uint16_t i = 0;
+    size_t i = 0;
     for (; i < respLen;) {
         if (AT91C_BASE_SSC->SSC_SR & (AT91C_SSC_TXRDY)) {
             AT91C_BASE_SSC->SSC_THR = resp[i++];
@@ -114,8 +114,10 @@ static int EmSendCmdThinfilmRaw(const uint8_t *resp, uint16_t respLen) {
     }
 
     // Ensure that the FPGA Delay Queue is empty
-    uint8_t fpga_queued_bits = FpgaSendQueueDelay >> 3;
-    for (i = 0; i <= fpga_queued_bits / 8u + 1u;) {
+    uint16_t fpga_queued_bits = FpgaSendQueueDelay >> 3;
+    fpga_queued_bits /= 8u;
+    fpga_queued_bits += 1u;
+    for (i = 0; i <= fpga_queued_bits;) {
         if (AT91C_BASE_SSC->SSC_SR & (AT91C_SSC_TXRDY)) {
             AT91C_BASE_SSC->SSC_THR = SEC_F;
             FpgaSendQueueDelay = (uint8_t)AT91C_BASE_SSC->SSC_RHR;