Prevent sector-unaligned erase

Author: Offir Kochalsky

components/storage/blockdevice/COMPONENT_SPIF/SPIFBlockDevice.cpp

@@ -339,12 +339,22 @@ int SPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
     int size = (int)in_size;
     bool erase_failed = false;
     int status = SPIF_BD_ERROR_OK;
-   // Find region of erased address
+    // Find region of erased address
     int region = _utils_find_addr_region(addr);
     // Erase Types of selected region
     uint8_t bitfield = _region_erase_types_bitfield[region];
 
-    tr_error("DEBUG: erase - addr: %llu, in_size: %llu", addr, in_size);
+    tr_info("DEBUG: erase - addr: %llu, in_size: %llu", addr, in_size);
+
+    if ((addr + in_size) > _device_size_bytes) {
+        tr_error("ERROR: erase exceeds flash device size");
+        return SPIF_BD_ERROR_INVALID_ERASE_PARAMS;
+    }
+
+    if ( ((addr % get_erase_size(addr)) != 0 ) ||  (((addr + in_size) % get_erase_size(addr + in_size - 1)) != 0 ) ) {
+        tr_error("ERROR: invalid erase - unaligned address and size");
+        return SPIF_BD_ERROR_INVALID_ERASE_PARAMS;
+    }
 
     // For each iteration erase the largest section supported by current region
     while (size > 0) {
@@ -353,13 +363,12 @@ int SPIFBlockDevice::erase(bd_addr_t addr, bd_size_t in_size)
         // find the matching instruction and erase size chunk for that type.
         type = _utils_iterate_next_largest_erase_type(bitfield, size, (unsigned int)addr, _region_high_boundary[region]);
         cur_erase_inst = _erase_type_inst_arr[type];
-        //chunk = _erase_type_size_arr[type];
         offset = addr % _erase_type_size_arr[type];
         chunk = ( (offset + size) < _erase_type_size_arr[type]) ? size : (_erase_type_size_arr[type] - offset);
 
-        tr_error("DEBUG: erase - addr: %llu, size:%d, Inst: 0x%xh, chunk: %lu , ",
+        tr_debug("DEBUG: erase - addr: %llu, size:%d, Inst: 0x%xh, chunk: %lu , ",
                  addr, size, cur_erase_inst, chunk);
-        tr_error("DEBUG: erase - Region: %d, Type:%d",
+        tr_debug("DEBUG: erase - Region: %d, Type:%d",
                  region, type);
 
         _mutex->lock();

components/storage/blockdevice/COMPONENT_SPIF/SPIFBlockDevice.h

@@ -25,11 +25,12 @@
  *  @enum spif_bd_error
  */
 enum spif_bd_error {
-    SPIF_BD_ERROR_OK                 = 0,     /*!< no error */
-    SPIF_BD_ERROR_DEVICE_ERROR       = BD_ERROR_DEVICE_ERROR, /*!< device specific error -4001 */
-    SPIF_BD_ERROR_PARSING_FAILED     = -4002, /* SFDP Parsing failed */
-    SPIF_BD_ERROR_READY_FAILED		 = -4003, /* Wait for  Mem Ready failed */
-    SPIF_BD_ERROR_WREN_FAILED        = -4004, /* Write Enable Failed */
+    SPIF_BD_ERROR_OK                    = 0,     /*!< no error */
+    SPIF_BD_ERROR_DEVICE_ERROR          = BD_ERROR_DEVICE_ERROR, /*!< device specific error -4001 */
+    SPIF_BD_ERROR_PARSING_FAILED        = -4002, /* SFDP Parsing failed */
+    SPIF_BD_ERROR_READY_FAILED	        = -4003, /* Wait for  Mem Ready failed */
+    SPIF_BD_ERROR_WREN_FAILED           = -4004, /* Write Enable Failed */
+    SPIF_BD_ERROR_INVALID_ERASE_PARAMS  = -4005, /* Erase command not on sector aligned addresses or exceeds device size */
 };
 
 
@@ -136,6 +137,7 @@ class SPIFBlockDevice : public BlockDevice {
      *                  SPIF_BD_ERROR_DEVICE_ERROR - device driver transaction failed
      *                  SPIF_BD_ERROR_READY_FAILED - Waiting for Memory ready failed or timed out
      *                  SPIF_BD_ERROR_WREN_FAILED - Write Enable failed
+     *                  SPIF_BD_ERROR_INVALID_ERASE_PARAMS - Trying to erase unaligned address or size
      */
     virtual int erase(bd_addr_t addr, bd_size_t size);
 

components/storage/blockdevice/COMPONENT_SPIF/TESTS/block_device/spif/main.cpp

@@ -131,9 +131,9 @@ void test_spif_random_program_read_erase()
     delete[] read_block;
 }
 
-void test_spif_unaligned_program()
+void test_spif_unaligned_erase()
 {
-    utest_printf("\nTest Unaligned Program Starts..\n");
+    utest_printf("\nTest Unaligned Erase Starts..\n");
 
     SPIFBlockDevice blockD(MBED_CONF_SPIF_DRIVER_SPI_MOSI, MBED_CONF_SPIF_DRIVER_SPI_MISO, MBED_CONF_SPIF_DRIVER_SPI_CLK,
                            MBED_CONF_SPIF_DRIVER_SPI_CS);
@@ -153,57 +153,42 @@ void test_spif_unaligned_program()
         }
     }
 
-    bd_size_t block_size = blockD.get_erase_size();
+    bd_addr_t addr = 0;
+    bd_size_t sector_erase_size = blockD.get_erase_size(addr);
     unsigned addrwidth = ceil(log(float(blockD.size() - 1)) / log(float(16))) + 1;
 
-    uint8_t *write_block = new (std::nothrow) uint8_t[block_size];
-    uint8_t *read_block = new (std::nothrow) uint8_t[block_size];
-    if (!write_block || !read_block ) {
-        utest_printf("\n Not enough memory for test");
-        goto end;
-    }
-
-    {
-        bd_addr_t block = (rand() * block_size) % blockD.size() + 1;
+    utest_printf("\ntest  %0*llx:%llu...", addrwidth, addr, sector_erase_size);
 
-        // Use next random number as temporary seed to keep
-        // the address progressing in the pseudorandom sequence
-        unsigned seed = rand();
+    //unaligned start address
+    addr += 1;
+    err = blockD.erase(addr, sector_erase_size - 1);
+    TEST_ASSERT_EQUAL(SPIF_BD_ERROR_INVALID_ERASE_PARAMS, err);
 
-        // Fill with random sequence
-        srand(seed);
-        for (bd_size_t i_ind = 0; i_ind < block_size; i_ind++) {
-            write_block[i_ind] = 0xff & rand();
-        }
+    err = blockD.erase(addr, sector_erase_size);
+    TEST_ASSERT_EQUAL(SPIF_BD_ERROR_INVALID_ERASE_PARAMS, err);
 
-        // Write, sync, and read the block
-        utest_printf("\ntest  %0*llx:%llu...", addrwidth, block, block_size);
+    err = blockD.erase(addr, 1);
+    TEST_ASSERT_EQUAL(SPIF_BD_ERROR_INVALID_ERASE_PARAMS, err);
 
-        err = blockD.erase(block, block_size);
-        TEST_ASSERT_EQUAL(0, err);
+    //unaligned end address
+    addr = 0;
 
-        //err = blockD.erase(block+4096, block_size);
-        //TEST_ASSERT_EQUAL(0, err);
+    err = blockD.erase(addr, 1);
+    TEST_ASSERT_EQUAL(SPIF_BD_ERROR_INVALID_ERASE_PARAMS, err);
 
-        err = blockD.program(write_block, block, block_size);
-        TEST_ASSERT_EQUAL(0, err);
+    err = blockD.erase(addr, sector_erase_size + 1);
+    TEST_ASSERT_EQUAL(SPIF_BD_ERROR_INVALID_ERASE_PARAMS, err);
 
-        err = blockD.read(read_block, block, block_size);
-        TEST_ASSERT_EQUAL(0, err);
+    //erase size exceeds flash device size
+    err = blockD.erase(addr, blockD.size() + 1);
+    TEST_ASSERT_EQUAL(SPIF_BD_ERROR_INVALID_ERASE_PARAMS, err);
 
-        // Check that the data was unmodified
-        srand(seed);
-        for (bd_size_t i_ind = 0; i_ind < block_size; i_ind++) {
-            //printf("index %d\n", i_ind);
-            TEST_ASSERT_EQUAL(0xff & rand(), read_block[i_ind]);
-        }
+    // Valid erase
+    err = blockD.erase(addr, sector_erase_size);
+    TEST_ASSERT_EQUAL(SPIF_BD_ERROR_OK, err);
 
-        err = blockD.deinit();
-        TEST_ASSERT_EQUAL(0, err);
-    }
-end:
-    delete[] write_block;
-    delete[] read_block;
+    err = blockD.deinit();
+    TEST_ASSERT_EQUAL(0, err);
 }
 
 static void test_spif_thread_job(void *vBlockD/*, int thread_num*/)
@@ -282,7 +267,7 @@ utest::v1::status_t test_setup(const size_t number_of_cases)
 }
 
 Case cases[] = {
-    Case("Testing unaligned program blocks", test_spif_unaligned_program),
+    Case("Testing unaligned erase blocks", test_spif_unaligned_erase),
     Case("Testing read write random blocks", test_spif_random_program_read_erase),
     Case("Testing Multi Threads Erase Program Read", test_spif_multi_threads)
 };