Improved flash image validation (#1916)

* fix_shift_back

* clean up

* gitignore

* remove the workaround in notpad cuz it's been fixed in this PR

* format

* add credit for mark

* 2024

.gitignore

@@ -68,9 +68,11 @@ CMakeFiles/
 .DS_Store
 /firmware/CMakeCache.txt
 
-# Python env/ venv
+# Python env/ venv and cache
 env/
 venv/
+**/__pycache__/
+*.pyc
 
 # Other
 *.bak

firmware/application/apps/ui_flash_utility.cpp

@@ -28,27 +28,49 @@ namespace ui {
 
 static const char16_t* firmware_folder = u"/FIRMWARE";
 
+// Firmware image validation
+static const char* hackrf_magic = "HACKRFFW";
+#define FIRMWARE_INFO_AREA_OFFSET 0x400
+#define FIRST_CHECKSUM_NIGHTLY 240125
+
 Thread* FlashUtilityView::thread{nullptr};
 static constexpr size_t max_filename_length = 26;
 
 bool valid_firmware_file(std::filesystem::path::string_type path) {
     File firmware_file;
+    bool require_checksum{false};
     uint32_t read_buffer[128];
-    uint32_t checksum{(uint32_t)~FLASH_EXPECTED_CHECKSUM};  // initializing to invalid checksum in case file can't be read
+    uint32_t checksum{FLASH_CHECKSUM_ERROR};  // initializing to invalid checksum in case file can't be read
+
+    static_assert((FIRMWARE_INFO_AREA_OFFSET % sizeof(read_buffer)) == 0, "Read buffer size must divide evenly into FIRMWARE_INFO_AREA_OFFSET");
 
     // test read of the whole file just to validate checksum (baseband flash code will re-read when flashing)
     auto result = firmware_file.open(path.c_str());
     if (!result.is_valid()) {
         checksum = 0;
-        for (uint32_t i = 0; i < FLASH_ROM_SIZE / sizeof(read_buffer); i++) {
+        for (uint32_t offset = 0; offset < FLASH_ROM_SIZE; offset += sizeof(read_buffer)) {
             auto readResult = firmware_file.read(&read_buffer, sizeof(read_buffer));
 
-            // if file is smaller than 1MB, assume it's a downgrade to an old FW version and ignore the checksum
             if ((!readResult) || (readResult.value() != sizeof(read_buffer))) {
-                checksum = FLASH_EXPECTED_CHECKSUM;
+                // File was smaller than 1MB:
+                // If version is such that the file SHOULD have been 1MB, call it a checksum error (otherwise say it's OK).
+                checksum = (require_checksum) ? FLASH_CHECKSUM_ERROR : FLASH_EXPECTED_CHECKSUM;
                 break;
             }
 
+            // Did we just read the firmware info area?
+            if (offset == FIRMWARE_INFO_AREA_OFFSET) {
+                // If there's no info area (missing HACKRFFW signature), it could be an ancient FW version (so skipping check)
+                if (memcmp(read_buffer, hackrf_magic, 8) == 0) {
+                    char* version_string = (char*)&read_buffer[4];
+
+                    // Require a 1MB firmware image with a valid checksum if release version >=v2.x or nightly >n_240125
+                    if (((version_string[0] == 'v') && (std::atoi(&version_string[1]) >= 2)) ||
+                        ((version_string[0] == 'n') && (version_string[1] == '_') && (std::atoi(&version_string[2]) >= FIRST_CHECKSUM_NIGHTLY)))
+                        require_checksum = true;
+                }
+            }
+
             checksum += simple_checksum((uint32_t)read_buffer, sizeof(read_buffer));
         }
     }

firmware/application/apps/ui_flash_utility.hpp

@@ -35,6 +35,7 @@
 #define FLASH_ROM_SIZE 1048576
 #define FLASH_STARTING_ADDRESS 0x00000000
 #define FLASH_EXPECTED_CHECKSUM 0x00000000
+#define FLASH_CHECKSUM_ERROR 0xFFFFFFFF
 
 namespace ui {
 

firmware/application/apps/ui_text_editor.cpp

@@ -27,8 +27,6 @@
 #include "log_file.hpp"
 #include "string_format.hpp"
 
-#include "portapack_persistent_memory.hpp"
-
 using namespace portapack;
 namespace fs = std::filesystem;
 
@@ -556,8 +554,6 @@ void TextEditorView::open_file(const fs::path& path) {
         viewer.set_file(*file_);
     }
 
-    portapack::persistent_memory::set_apply_fake_brightness(false);  // work around to resolve the display issue in notepad app. not elegant i know, so TODO.
-
     refresh_ui();
 }
 

firmware/application/usb_serial_shell_filesystem.cpp

@@ -26,6 +26,8 @@
 #include "string_format.hpp"
 #include <cstring>
 
+#include "crc.hpp"
+
 static File* shell_file = nullptr;
 
 static bool report_on_error(BaseSequentialStream* chp, File::Error& error) {
@@ -371,3 +373,34 @@ void cmd_sd_write_binary(BaseSequentialStream* chp, int argc, char* argv[]) {
 
     chprintf(chp, "ok\r\n");
 }
+
+void cmd_sd_crc32(BaseSequentialStream* chp, int argc, char* argv[]) {
+    if (argc != 1) {
+        chprintf(chp, "usage: crc32 <path>\r\n");
+        return;
+    }
+
+    auto path = path_from_string8(argv[0]);
+    File* crc_file = new File();
+    auto error = crc_file->open(path, true, false);
+    if (report_on_error(chp, error)) return;
+
+    uint8_t buffer[64];
+    CRC<32> crc{0x04c11db7, 0xffffffff, 0xffffffff};
+
+    while (true) {
+        auto bytes_read = crc_file->read(buffer, 64);
+        if (report_on_error(chp, bytes_read)) return;
+
+        if (bytes_read.value() > 0) {
+            crc.process_bytes((void*)buffer, bytes_read.value());
+        }
+
+        if (64 != bytes_read.value()) {
+            chprintf(chp, "CRC32: 0x%08X\r\n", crc.checksum());
+            return;
+        }
+    }
+
+    delete crc_file;
+}

firmware/application/usb_serial_shell_filesystem.hpp

@@ -42,6 +42,7 @@ void cmd_sd_read(BaseSequentialStream* chp, int argc, char* argv[]);
 void cmd_sd_read_binary(BaseSequentialStream* chp, int argc, char* argv[]);
 void cmd_sd_write(BaseSequentialStream* chp, int argc, char* argv[]);
 void cmd_sd_write_binary(BaseSequentialStream* chp, int argc, char* argv[]);
+void cmd_sd_crc32(BaseSequentialStream* chp, int argc, char* argv[]);
 
 static std::filesystem::path path_from_string8(char* path) {
     std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t> conv;
@@ -62,6 +63,7 @@ static std::filesystem::path path_from_string8(char* path) {
     {"ftell", cmd_sd_tell},            \
     {"fread", cmd_sd_read},            \
     {"frb", cmd_sd_read_binary},       \
-    {"fwrite", cmd_sd_write},           \
-    {"fwb", cmd_sd_write_binary}
+    {"fwrite", cmd_sd_write},          \
+    {"fwb", cmd_sd_write_binary},      \
+    {"crc32", cmd_sd_crc32}
 // clang-format on

firmware/common/portapack_io.hpp

@@ -2,6 +2,7 @@
  * Copyright (C) 2014 Jared Boone, ShareBrained Technology, Inc.
  * Copyleft (ɔ) 2024 zxkmm under GPL license
  * Copyright (C) 2024 u-foka
+ * Copyright (C) 2024 Mark Thompson
  *
  * This file is part of PortaPack.
  *
@@ -34,6 +35,21 @@
 
 // #include "portapack_persistent_memory.hpp"
 
+// Darkened pixel bit mask for each possible shift value.
+static const uint16_t darken_mask[4] = {
+    0b1111111111111111,  // RrrrrGgggggBbbbb
+    0b0111101111101111,  // 0Rrrr0Ggggg0Bbbb
+    0b0011100111100111,  // 00Rrr00Gggg00Bbb
+    0b0001100011100011   // 000Rr000Ggg000Bb
+};
+
+// To darken, dividing each color level R/G/B by 2^shift.
+#define DARKENED_PIXEL(pixel, shift) ((pixel >> shift) & darken_mask[shift])
+
+// To un-darken, multiply each color level by 2^shift (might still be darker that before since some bits may have been lost above).
+// This function will only be called when the pixel has previously been darkened, so no masking is needed.
+#define UNDARKENED_PIXEL(pixel, shift) (pixel << shift)
+
 namespace portapack {
 
 class IO {
@@ -147,7 +163,7 @@ class IO {
 
     void lcd_write_pixel(ui::Color pixel) {
         if (get_dark_cover()) {
-            darken_color(pixel, get_brightness());  // Darken the pixel color
+            pixel.v = DARKENED_PIXEL(pixel.v, get_brightness());
         }
         lcd_write_data(pixel.v);
     }
@@ -158,7 +174,7 @@ class IO {
 
     void lcd_write_pixels(ui::Color pixel, size_t n) {
         if (get_dark_cover()) {
-            darken_color(pixel, get_brightness());  // Darken the pixel color
+            pixel.v = DARKENED_PIXEL(pixel.v, get_brightness());
         }
         while (n--) {
             lcd_write_data(pixel.v);
@@ -167,7 +183,7 @@ class IO {
 
     void lcd_write_pixels_unrolled8(ui::Color pixel, size_t n) {
         if (get_dark_cover()) {
-            darken_color(pixel, get_brightness());  // Darken the pixel color
+            pixel.v = DARKENED_PIXEL(pixel.v, get_brightness());
         }
         auto v = pixel.v;
         n >>= 3;
@@ -331,25 +347,6 @@ class IO {
         addr(1); /* Set up for data phase (most likely after a command) */
     }
 
-    void darken_color(ui::Color& pixel, uint8_t darken_level_shift) {
-        // TODO: 1. do we need edge control?
-        // currently didn't see and issue without edge control
-        // but maybe hurts screen hardware without one?
-
-        // TODO: 2. de-color mode for accessibility
-        // TODO: 3. high contrast mode for accessibility
-
-        uint16_t r = (pixel.v >> 11) & 0x1F;  // Extract red
-        uint16_t g = (pixel.v >> 5) & 0x3F;   // Extract green
-        uint16_t b = pixel.v & 0x1F;          // Extract blue
-
-        r = r >> darken_level_shift;  // Darken red
-        g = g >> darken_level_shift;  // Darken green
-        b = b >> darken_level_shift;  // Darken blue
-
-        pixel.v = (r << 11) | (g << 5) | b;  // Combine back to color, check UI::color for the color layout
-    }
-
     // void high_contrast(ui::Color& pixel, size_t contrast_level_shift) {  // TODO
     //     uint16_t r = (pixel.v >> 11) & 0x1F;
     //     uint16_t g = (pixel.v >> 5) & 0x3F;
@@ -417,7 +414,12 @@ class IO {
         halPolledDelay(18);  // 90ns
 
         const auto value_low = data_read();
-        return (value_high << 8) | value_low;
+        uint32_t original_value = (value_high << 8) | value_low;
+
+        if (get_dark_cover()) {
+            original_value = UNDARKENED_PIXEL(original_value, get_brightness());
+        }
+        return original_value;
     }
 
     void io_write(const bool address, const uint_fast16_t value) {

sdcard/FREQMAN/RECON.TXT

@@ -1,22 +1,22 @@
-a=150000,b=285000,m=AM,bw=DSB,s=5kHz,d=Longwave broadcast band|BCB (EU)
-a=525000,b=1605000,m=AM,bw=DSB,s=5kHz,d=AM broadcast band|BCB (EU & J)
-a=530000,b=1710000,m=AM,bw=DSB,s=5kHz,d=AM broadcast band|BCB (US)
-a=1800000,b=29700000,m=WFM,bw=16k,d=Amateur_radio|Amateur
-a=26900000,b=27400000,m=WFM,bw=16k,d=Citizens_band_radio|Citizens band
-a=28000000,b=30000000,m=WFM,bw=16k,d=Amateur_radio|Amateur
-a=29000000,b=54000000,m=WFM,bw=16k,d=Land mobile
-a=50000000,b=54000000,m=WFM,bw=16k,d=Amateur_radio|Amateur
-a=65000000,b=85000000,m=WFM,bw=16k,d=Land mobile (EU)
-a=108000000,b=136000000,m=WFM,bw=16k,d=Aircraft
-a=120000000,b=160000000,m=WFM,bw=16k,s=50kHz,d=Land mobile (EU)
-a=132000000,b=174000000,m=WFM,bw=16k,s=50kHz,d=Land mobile
-a=142000000,b=170000000,m=WFM,bw=16k,s=50kHz,d=Land mobile (J)
-a=144000000,b=148000000,m=WFM,bw=16k,s=50kHz,d=Amateur_radio|Amateur
-a=216000000,b=222000000,m=WFM,bw=16k,s=50kHz,d=Land mobile
-a=222000000,b=225000000,m=WFM,bw=16k,s=50kHz,d=Amateur_radio|Amateur
-a=335000000,b=384000000,m=WFM,bw=16k,s=50kHz,d=Land mobile (J)
-a=406000000,b=512000000,m=WFM,bw=16k,s=50kHz,d=Land mobile
-a=450000000,b=470000000,m=WFM,bw=16k,s=50kHz,d=Land mobile (J)
-a=430000000,b=450000000,m=WFM,bw=16k,s=50kHz,d=Amateur_radio|Amateur
-a=806000000,b=947000000,m=WFM,bw=16k,s=50kHz,d=Land mobile
-a=1200000000,b=1600000000,m=WFM,bw=16k,s=50kHz,d=Amateur|Land mobile|GPS
+a=150000,b=285000,m=AM,bw=DSB 9k,s=5kHz,d=Longwave broadcast band|BCB (EU)
+a=525000,b=1605000,m=AM,bw=DSB 9k,s=5kHz,d=AM broadcast band|BCB (EU & J)
+a=530000,b=1710000,m=AM,bw=DSB 9k,s=5kHz,d=AM broadcast band|BCB (US)
+a=1800000,b=29700000,m=AM,bw=LSB-3k,d=Amateur_radio|Amateur
+a=26900000,b=27400000,m=AM,bw=DSB 9k,d=Citizens_band_radio|Citizens band
+a=28000000,b=30000000,m=AM,bw=USB+3k,d=Amateur_radio|Amateur
+a=29000000,b=54000000,m=AM,bw=DSB 9k,d=Land mobile
+a=50000000,b=54000000,m=NFM,bw=16k,d=Amateur_radio|Amateur
+a=65000000,b=85000000,m=NFM,bw=16k,d=Land mobile (EU)
+a=108000000,b=136000000,m=NFM,bw=16k,d=Aircraft
+a=120000000,b=160000000,m=NFM,bw=16k,s=50kHz,d=Land mobile (EU)
+a=132000000,b=174000000,m=NFM,bw=16k,s=50kHz,d=Land mobile
+a=142000000,b=170000000,m=NFM,bw=16k,s=50kHz,d=Land mobile (J)
+a=144000000,b=148000000,m=NFM,bw=16k,s=50kHz,d=Amateur_radio|Amateur
+a=216000000,b=222000000,m=NFM,bw=16k,s=50kHz,d=Land mobile
+a=222000000,b=225000000,m=NFM,bw=16k,s=50kHz,d=Amateur_radio|Amateur
+a=335000000,b=384000000,m=NFM,bw=16k,s=50kHz,d=Land mobile (J)
+a=406000000,b=512000000,m=NFM,bw=16k,s=50kHz,d=Land mobile
+a=450000000,b=470000000,m=NFM,bw=16k,s=50kHz,d=Land mobile (J)
+a=430000000,b=450000000,m=NFM,bw=16k,s=50kHz,d=Amateur_radio|Amateur
+a=806000000,b=947000000,m=NFM,bw=16k,s=50kHz,d=Land mobile
+a=1200000000,b=1600000000,m=NFM,bw=16k,s=50kHz,d=Amateur|Land mobile|GPS