Debugging round 1

.coverity.conf.sample

@@ -4,9 +4,9 @@ COVTOKEN=aAbBcCdDeEfFgGhHiIjJkK
 COVBINDIR="/opt/cov-analysis-linux64-2020.09/bin"
 # Nickname included in scan description:
 NICKNAME=myself
-HOSTCC=gcc-10
-HOSTCXX=g++-10
-HOSTLD=g++-10
+HOSTCC=cc
+HOSTCXX=c++
+HOSTLD=c++
 
 # Do not change it:
 COVDIR=cov-int

Makefile.defs

@@ -38,8 +38,8 @@ TAR = tar
 TARFLAGS ?= -v --ignore-failed-read -r
 TARFLAGS += -C .. -f
 CROSS ?= arm-none-eabi-
-CC ?= gcc
-CXX ?= g++
+CC ?= cc
+CXX ?= c++
 SH = sh
 BASH = bash
 PERL = perl
@@ -123,9 +123,9 @@ ifeq ($(DEBUG_ARM),1)
 endif
 # Next ones are activated only if SANITIZE=1
 ifeq ($(SANITIZE),1)
-  DEFCFLAGS += -g -fsanitize=address -fno-omit-frame-pointer
-  DEFCXXFLAGS += -g -fsanitize=address -fno-omit-frame-pointer
-  DEFLDFLAGS += -g -fsanitize=address
+  DEFCFLAGS += -g -fsanitize=undefined,address -fno-omit-frame-pointer
+  DEFCXXFLAGS += -g -fsanitize=undefined,address -fno-omit-frame-pointer
+  DEFLDFLAGS += -g -fsanitize=undefined,address
 endif
 # Some more warnings we want as errors:
 DEFCFLAGS += -Wbad-function-cast -Wredundant-decls -Wmissing-prototypes -Wchar-subscripts -Wshadow -Wundef -Wwrite-strings -Wunused -Wuninitialized -Wpointer-arith -Winline -Wformat -Wformat-security -Winit-self -Wmissing-include-dirs -Wnested-externs -Wmissing-declarations -Wempty-body -Wignored-qualifiers -Wmissing-field-initializers -Wtype-limits -Wold-style-definition

armsrc/Makefile

@@ -138,7 +138,6 @@ THUMBSRC = start.c \
     commonutil.c \
     util.c \
     string.c \
-    BigBuf.c \
     ticks.c \
     clocks.c \
     hfsnoop.c \

armsrc/appmain.c

@@ -67,10 +67,6 @@
 #include "sam_seos.h"
 #include "sam_mfc.h"
 
-#ifdef WITH_LCD
-#include "LCD_disabled.h"
-#endif
-
 #ifdef WITH_SMARTCARD
 #include "i2c.h"
 #endif

armsrc/cardemu.h

@@ -19,7 +19,7 @@
 #ifndef CARDEMU_H__
 #define CARDEMU_H__
 
-#include "common.h"
+#include "util.h"
 #include "palloc.h"
 
 #define CARD_MEMORY_SIZE 4096 // 4Kb should be a good size

armsrc/frozen.c

@@ -979,10 +979,10 @@ static void json_scanf_cb(void *callback_data, const char *name,
         case 'H': {
 #if JSON_ENABLE_HEX
             char **dst = (char **) info->user_data;
-            int i, len = token->len / 2;
+            int len = token->len / 2;
             *(int *) info->target = len;
             if ((*dst = (char *) malloc(len + 1)) != NULL) {
-                for (i = 0; i < len; i++) {
+                for (int i = 0; i < len; i++) {
                     (*dst)[i] = hexdec(token->ptr + 2 * i);
                 }
                 (*dst)[len] = '\0';

armsrc/hitagS.c

@@ -1250,11 +1250,11 @@ static int selectHitagS(const lf_hitag_data_t *packet, uint8_t *tx, size_t sizeo
 
     //check which memorysize this tag has
     if ((conf_pages[0] & 0x3) == 0x00) {
-        tag.max_page = 32 / 32;
+        tag.max_page = 1; // Since 32 / 32 == 1
     } else if ((conf_pages[0] & 0x3) == 0x1) {
-        tag.max_page = 256 / 32;
+        tag.max_page = 8; // Since 256 / 32 == 8
     } else if ((conf_pages[0] & 0x3) == 0x2) {
-        tag.max_page = 2048 / 32;
+        tag.max_page = 64; // Since 2048 / 32 == 64
     }
 
     conf_pages[1] = rx[1];

armsrc/lfsampling.h

@@ -16,7 +16,7 @@
 #ifndef __LFSAMPLING_H
 #define __LFSAMPLING_H
 
-#include "common.h"
+#include "util.h"
 #include "pm3_cmd.h"
 
 typedef struct {

armsrc/palloc.c

@@ -22,8 +22,13 @@
 //==============================================================================
 #include "palloc.h"
 
-#include "util.h"    // nbytes
+#ifndef offsetof
+#define offsetof(type, field) ((size_t) &(((type *) 0)->field))
+#endif
+
 #include "dbprint.h" // logging
+#include "proxmark3_arm.h" // LED control
+#include "ticks.h"
 #include "pm3_cmd.h" // return defines
 
 // Word size alignment
@@ -42,20 +47,20 @@ extern uint32_t _stack_start[], __bss_end__[];
 #define MAX_BLOCKS 32 // 32 blocks should give us an overall overhead of 768 bytes
 
 typedef struct Block pBlock;
+typedef struct Heap pHeap;
 
-struct Block {
-    void *address; // The memory address this block points to
-    pBlock *next;  // The next block in the list, or nullptr if there is none
+struct PACKED Block {
     int16_t size;  // The size of the data at `address`
+    void *address; // The memory address this block points to
+    struct Block *next;  // The next block in the list, or nullptr if there is none
 };
 
-typedef struct {
-    bool init;     // Flag for if the heap was initialized
+struct PACKED Heap {
+    pBlock *fresh; // Fresh (never used) Blocks List
     pBlock *free;  // Free (previously used) Blocks List
     pBlock *used;  // Currently used Blocks List
-    pBlock *fresh; // Fresh (never used) Blocks List
     size_t top;    // Top free address
-} pHeap;
+};
 
 /**
  * @brief The FPGA Queue
@@ -84,25 +89,27 @@ static size_t free_space = 0;
 void palloc_init(void) {
     // Set up the heap
     heap = (pHeap*)(_stack_start - __bss_end__);
-    heap->init = false; // Signal that we haven't finished initializing yet
     heap->free = nullptr;
     heap->used = nullptr;
-    heap->fresh = (pBlock*)(heap + 1);
-    heap->top = (size_t)(heap->fresh + MAX_BLOCKS);
+    heap->fresh = (pBlock*)(heap + sizeof(memptr_t));
+    heap->top = (size_t)(heap->fresh + (MAX_BLOCKS * sizeof(memptr_t)));
 
     // Set up the fresh blocks to use
     pBlock *block = heap->fresh;
     uint8_t i = (MAX_BLOCKS - 1);
     while(i--) {
-        block->next = block + 1;
+        LED_D_INV();
+        SpinDelay(100);
+        block->next = (pBlock*)(block + sizeof(memptr_t));
+        block->size = 0;
         block++;
     }
 
     // Calculate the amount of free space we have in the heap
     free_space = (MEM_USABLE - OVERHEAD);
 
+    block->size = 0;
     block->next = nullptr; // Set the last next block to nullptr to signal end of list
-    heap->init = true; // Signal that we have initialized the heap
 }
 
 /**
@@ -254,7 +261,7 @@ static pBlock *allocate_block(size_t alloc) {
  * @return the address of the block of memory, or nullptr
  */
 memptr_t *palloc(uint16_t numElement, const uint16_t size) {
-    if(((heap == NULL) || !(heap->init))) return false; // Can't allocate memory if we haven't initialized any
+    if(heap == nullptr) return false; // Can't allocate memory if we haven't initialized any
 
     size_t orig = numElement;
     numElement *= size;
@@ -350,7 +357,7 @@ bool palloc_free(void *ptr) {
  * @return false otherwise
  */
 bool palloc_freeEX(void *ptr, bool verbose) {
-    if(((heap == NULL) || !(heap->init))) return false; // Can't free memory if we haven't initialized any
+    if(heap == NULL) return false; // Can't free memory if we haven't initialized any
 
     pBlock *blk = heap->used;
     pBlock *prev = nullptr;
@@ -384,15 +391,25 @@ bool palloc_freeEX(void *ptr, bool verbose) {
  * @param ptr The pointer of the Block container to count
  * @return The amount of blocks in that container (`int8_t`) or -1 for uninitialized heap
  */
-static int8_t count_blocks(pBlock *ptr) {
-    if(((heap == NULL) || !(heap->init))) return -1; // Can't count blocks if we don't have any
-    int8_t count = 0;
+static int count_blocks(pBlock *ptr) {
+    if(heap == nullptr) return -1;
+    Dbprintf("Got here");
 
-    while(ptr != nullptr) {
+    int count = 0;
+
+    while(count < MAX_BLOCK_SIZE) {
         count++;
-        ptr = ptr->next;
+
+        pBlock *blk = ptr->next;
+        if(blk == nullptr) break;
+
+        Dbprintf("count: %i", count);
+
+        ptr = blk->next;
     }
 
+    Dbprintf("count: %i", count);
+
     return count;
 }
 
@@ -402,7 +419,7 @@ static int8_t count_blocks(pBlock *ptr) {
  * 
  * @return The number of free blocks in the Heap, or -1 if the heap hasn't been initialized
  */
-int8_t palloc_free_blocks(void) {
+int palloc_free_blocks(void) {
     return count_blocks(heap->free);
 }
 
@@ -411,7 +428,7 @@ int8_t palloc_free_blocks(void) {
  * 
  * @return The number of used Blocks in the Heap, or -1 if the heap hasn't been initialized
  */
-int8_t palloc_used_blocks(void) {
+int palloc_used_blocks(void) {
     return count_blocks(heap->used);
 }
 
@@ -420,7 +437,7 @@ int8_t palloc_used_blocks(void) {
  * 
  * @return The number of fresh Blocks in the Heap, or -1 if the heap hasn't been initialized
  */
-int8_t palloc_fresh_blocks(void) {
+int palloc_fresh_blocks(void) {
     return count_blocks(heap->fresh);
 }
 
@@ -439,7 +456,7 @@ size_t palloc_sram_left(void) {
  * dire situations, it might be useful to manually do it.
  */
 void palloc_compact_heap(void) {
-    if(((heap == NULL) || !(heap->init))) return; // Sanity checking
+    if(heap == nullptr) return; // Sanity checking
 
     compact_heap();
 }
@@ -451,21 +468,30 @@ void palloc_compact_heap(void) {
  * @return `false` otherwise
  */
 bool palloc_heap_integrity(void) {
-    return (MAX_BLOCKS == palloc_free_blocks() + palloc_fresh_blocks() + palloc_used_blocks());
+    int count = 0;
+
+    Dbprintf("Counting Fresh blocks... (Heap offset: %u)", offsetof(pHeap, fresh));
+    count += palloc_fresh_blocks();
+    Dbprintf("Counting Free blocks.... (Heap offset: %u)", offsetof(pHeap, free));
+    //count += palloc_free_blocks();
+    Dbprintf("Counting Used blocks.... (Heap offset: %u)", offsetof(pHeap, used));
+    //count += palloc_used_blocks();
+
+    return MAX_BLOCKS == count;
 }
 
 void palloc_status(void) {
     Dbprintf("--- " _CYAN_("Memory") " -----------------");
-    Dbprintf(" - Usuable:................ "_CYAN_("%d"), MEM_USABLE);
-    Dbprintf(" - Free:................... "_CYAN_("%d"), palloc_sram_left());
-    Dbprintf(" - Heap Status:............ %s",
-        (palloc_heap_integrity() ? _GREEN_("OK") : _RED_("INTEGRITY ISSUES"))
-    );
+    Dbprintf(" - Heap Top:............... "_YELLOW_("0x%x"), heap->top);
+    Dbprintf(" - Usable:................. "_YELLOW_("%d"), MEM_USABLE);
+    Dbprintf(" - Free:................... "_YELLOW_("%d"), palloc_sram_left());
+    Dbprintf(" - Heap Initialized:....... %s", (heap != nullptr ? _GREEN_("YES") : _RED_("NO")));
+    Dbprintf(" - Heap Status:............ %s", (palloc_heap_integrity() ? _GREEN_("OK") : _RED_("INTEGRITY ISSUES")));
 
     Dbprintf("--- " _CYAN_("Blocks") " -----------------");
-    Dbprintf(" - Fresh:.................. "_CYAN_("%d"), palloc_fresh_blocks());
-    Dbprintf(" - Used:................... "_CYAN_("%d"), palloc_used_blocks());
-    Dbprintf(" - Free:................... "_CYAN_("%d"), palloc_free_blocks());
+    Dbprintf(" - Fresh:.................. "_YELLOW_("%d"), palloc_fresh_blocks());
+    Dbprintf(" - Used:................... "_YELLOW_("%d"), palloc_used_blocks());
+    Dbprintf(" - Free:................... "_YELLOW_("%d"), palloc_free_blocks());
 }
 
 uint32_t palloc_sram_size() {
@@ -481,7 +507,7 @@ uint32_t palloc_sram_size() {
 buffer8u_t palloc_buffer8(uint16_t numElement) {
     buffer8u_t buffer = { .data = nullptr, .size = 0 }; // initialize a "empty" buffer
 
-    if(((heap == NULL) || !(heap->init)) || numElement > MAX_BLOCK_SIZE) return buffer; // Sanity checking
+    if(heap == nullptr || numElement > MAX_BLOCK_SIZE) return buffer; // Sanity checking
 
     if(numElement & ALIGN_MASK) { // Make sure we align our sizes
         numElement = (numElement + ALIGN_BYTES - 1) & ~ALIGN_MASK;
@@ -510,7 +536,7 @@ buffer16u_t palloc_buffer16(uint16_t numElement) {
     buffer16u_t buffer = { .data = nullptr, .size = 0 }; // initialize a "empty" buffer
     size_t alloc = numElement * sizeof(uint16_t); // Adjust for the buffer type
 
-    if(((heap == NULL) || !(heap->init)) || alloc > MAX_BLOCK_SIZE) return buffer; // Sanity checking
+    if(heap == nullptr || alloc > MAX_BLOCK_SIZE) return buffer; // Sanity checking
 
     if(alloc & ALIGN_MASK) { // Make sure we align our sizes
         alloc = (alloc + ALIGN_BYTES - 1) & ~ALIGN_MASK;
@@ -539,7 +565,7 @@ buffer32u_t palloc_buffer32(uint16_t numElement) {
     buffer32u_t buffer = { .data = nullptr, .size = 0 }; // initialize a "empty" buffer
     size_t alloc = numElement * sizeof(uint32_t); // Adjust for the buffer type
 
-    if(((heap == NULL) || !(heap->init)) || alloc > MAX_BLOCK_SIZE) return buffer; // Sanity checking
+    if(heap == nullptr || alloc > MAX_BLOCK_SIZE) return buffer; // Sanity checking
 
     if(alloc & ALIGN_MASK) { // Make sure we align our sizes
         alloc = (alloc + ALIGN_BYTES - 1) & ~ALIGN_MASK;

armsrc/palloc.h

@@ -22,7 +22,7 @@
 #ifndef PALLOC_H__
 #define PALLOC_H__
 
-#include "common.h"
+#include "util.h"
 
 //-----------------------------------------------------------------------------
 // Palloc (Proxmark ALLOCator) provides bare metal access to the memory
@@ -49,9 +49,9 @@ void palloc_set(void *ptr, const uint16_t value, uint16_t len);
 bool palloc_free(void *ptr);
 bool palloc_freeEX(void *ptr, bool verbose);
 
-int8_t palloc_free_blocks(void);
-int8_t palloc_used_blocks(void);
-int8_t palloc_fresh_blocks(void);
+int palloc_free_blocks(void);
+int palloc_used_blocks(void);
+int palloc_fresh_blocks(void);
 size_t palloc_sram_left(void);
 void palloc_compact_heap(void);
 bool palloc_heap_integrity(void);
@@ -68,17 +68,17 @@ uint32_t palloc_sram_size(void);
 typedef struct { // General purpose 8-bit buffer
     uint16_t size;
     uint8_t *data; // Pass this into `palloc_free()` to free the buffer
-} buffer8u_t;
+} PACKED buffer8u_t;
 
 typedef struct { // General purpose 16-bit buffer
     uint16_t size;
     uint16_t *data; // Pass this into `palloc_free()` to free the buffer
-} buffer16u_t;
+} PACKED buffer16u_t;
 
 typedef struct { // General purpose 32-bit buffer
     uint16_t size;
     uint32_t *data; // Pass this into `palloc_free()` to free the buffer
-} buffer32u_t;
+} PACKED buffer32u_t;
 
 buffer8u_t  palloc_buffer8(uint16_t numElement);
 buffer16u_t palloc_buffer16(uint16_t numElement);
@@ -95,7 +95,7 @@ typedef struct {
     int16_t max; // -1 is no data, max data size is ~2.3k bytes
     uint8_t bit; // 0 through 8
     uint8_t *data;
-} fpga_queue_t;
+} PACKED fpga_queue_t;
 
 fpga_queue_t *get_fpga_queue(void);
 void reset_fpga_queue(void);

armsrc/tracer.h

@@ -22,7 +22,7 @@
 #ifndef TRACER_H__
 #define TRACER_H__
 
-#include "common.h"
+#include "util.h"
 
 //-----------------------------------------------------------------------------
 // Tracer is the next iteration of the Proxmark Tracing functions.