Skip to content

Commit

Permalink
fix(espcoredump): Ensure consistency between the flash and checksum data
Browse files Browse the repository at this point in the history
This is a direct core dump of the current memory so it may change because
of other interrupt handlers or the memory used by the core dump process
itself. Always write the data via the cache buffer so that what's written
matches the checksum.

Fixes part of espressif/esp-zigbee-sdk#326 (invalid CRC when writing a core
dump to flash).
  • Loading branch information
nomis committed Jun 4, 2024
1 parent 636ff35 commit 3810de6
Showing 1 changed file with 5 additions and 48 deletions.
53 changes: 5 additions & 48 deletions components/espcoredump/src/core_dump_flash.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,20 +154,18 @@ static esp_err_t esp_core_dump_flash_write_data(core_dump_write_data_t* wr_data,
uint32_t wr_sz = 0;

/* Make sure that the partition is large enough to hold the data. */
ESP_COREDUMP_ASSERT((wr_data->off + data_size) < s_core_flash_config.partition.size);
ESP_COREDUMP_ASSERT((wr_data->off + wr_data->cached_bytes + data_size) < s_core_flash_config.partition.size);

if (wr_data->cached_bytes) {
/* Some bytes are in the cache, let's continue filling the cache
* with the data received as parameter. Let's calculate the maximum
* amount of bytes we can still fill the cache with. */
while (data_size > 0) {
/* Calculate the maximum amount of bytes we can still fill the cache with. */
if ((COREDUMP_CACHE_SIZE - wr_data->cached_bytes) > data_size) {
wr_sz = data_size;
} else {
wr_sz = COREDUMP_CACHE_SIZE - wr_data->cached_bytes;
}

/* Append wr_sz bytes from data parameter to the cache. */
memcpy(&wr_data->cached_data[wr_data->cached_bytes], data, wr_sz);
memcpy(&wr_data->cached_data[wr_data->cached_bytes], data + written, wr_sz);
wr_data->cached_bytes += wr_sz;

if (wr_data->cached_bytes == COREDUMP_CACHE_SIZE) {
Expand Down Expand Up @@ -195,47 +193,6 @@ static esp_err_t esp_core_dump_flash_write_data(core_dump_write_data_t* wr_data,
data_size -= wr_sz;
}

/* Figure out how many bytes we can write onto the flash directly, without
* using the cache. In our case the cache size is a multiple of the flash's
* minimum writing block size, so we will use it for our calculation.
* For example, if COREDUMP_CACHE_SIZE equals 32, here are interesting
* values:
* +---------+-----------------------+
* | | data_size |
* +---------+---+----+----+----+----+
* | | 0 | 31 | 32 | 40 | 64 |
* +---------+---+----+----+----+----+
* | (blocks | 0 | 0 | 1 | 1 | 2) |
* +---------+---+----+----+----+----+
* | wr_sz | 0 | 0 | 32 | 32 | 64 |
* +---------+---+----+----+----+----+
*/
wr_sz = (data_size / COREDUMP_CACHE_SIZE) * COREDUMP_CACHE_SIZE;
if (wr_sz) {
/* Write the contiguous amount of bytes to the flash,
* without using the cache */
err = esp_core_dump_flash_custom_write(s_core_flash_config.partition.start + wr_data->off, data + written, wr_sz);

if (err != ESP_OK) {
ESP_COREDUMP_LOGE("Failed to write data to flash (%d)!", err);
return err;
}

/* Update the checksum with the newly written bytes */
esp_core_dump_checksum_update(&wr_data->checksum_ctx, data + written, wr_sz);
wr_data->off += wr_sz;
written += wr_sz;
data_size -= wr_sz;
}

if (data_size > 0) {
/* There still some bytes from the data parameter that need to be sent,
* append it to cache in order to write them later. (i.e. when there
* will be enough bytes to fill the cache) */
memcpy(&wr_data->cached_data, data + written, data_size);
wr_data->cached_bytes = data_size;
}

return ESP_OK;
}

Expand All @@ -258,7 +215,7 @@ static esp_err_t esp_core_dump_flash_write_prepare(core_dump_write_data_t *wr_da
padding = COREDUMP_CACHE_SIZE - modulo;
}

/* Now we can check whether we have enough space in our core dump parition
/* Now we can check whether we have enough space in our core dump partition
* or not. */
if ((*data_len + padding + cs_len) > s_core_flash_config.partition.size) {
ESP_COREDUMP_LOGE("Not enough space to save core dump!");
Expand Down

0 comments on commit 3810de6

Please sign in to comment.