Skip to content

Process out-of-order vfs sectors for Universal Hex in block format #1092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions source/board/microbitv2/microbitv2.c
Original file line number Diff line number Diff line change
Expand Up @@ -446,13 +446,16 @@ static uint32_t read_file_data_txt(uint32_t sector_offset, uint8_t *data, uint32
return VFS_SECTOR_SIZE;
}

uint8_t board_detect_incompatible_image(const uint8_t *data, uint32_t size)
uint8_t board_detect_incompatible_image(uint32_t addr, const uint8_t *data, uint32_t size)
{
uint8_t result = 0;
// We can only check image compatibility on the first block of flash data
if (addr != 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first block of flash data is not always at address 0x0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, this didn't meant the first block received that contains flash data, but that the check should only be performed on the block that contains the first 512 bytes of flash data, which might arrive at any point.
I can update the comment to make this more clear.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think addr is not the offset in the image, but the address at which the block is intended to be flashed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I'll need to double check this, but my assumption here is that as this is being called within flash_decoder_write(addr, data, size), that would be the address in the target flash where the data block will be written.
So, this function is meant to let pass through all other data blocks as being compatible, and only perform the check on the raw data going to flash address 0x0. At that point it checks the vector table to see if it's an M4 or M0+ image and if it fails the check it makes DAPLink stop flashing and throw an error.

This is really only useful for Intel Hex files (not for Universal or UF2), to detect an old V1 Intel Hex file being flashed into a V2. Although it could also be useful for UF2 files without the family ID, but realistically the micro:bit editors will not really produce these, so those might only be produced by other tools, and those users are likely to be more advanced.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed that it was in source/board/microbitv2/microbitv2.c.

I was expecting to check against something like g_board_info.target_cfg.flash_regions[0].start which in your case is always 0.

return 0;
}

// Check difference in vectors (mem fault, bus fault, usage fault)
// If these vectors are 0, we assume it's an M0 image (not compatible)
result = memcmp(data + M0_RESERVED_VECT_OFFSET, (uint8_t[M0_RESERVED_VECT_SIZE]){0}, M0_RESERVED_VECT_SIZE);
uint8_t result = memcmp(data + M0_RESERVED_VECT_OFFSET, (uint8_t[M0_RESERVED_VECT_SIZE]){0}, M0_RESERVED_VECT_SIZE);

return result == 0;
}
Expand Down
100 changes: 98 additions & 2 deletions source/daplink/drag-n-drop/file_stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@
#include "compiler.h"
#include "validation.h"

// Set to 1 to enable debugging
#define DEBUG_FILE_STREAM 0

#if DEBUG_FILE_STREAM
#include "daplink_debug.h"
#define stream_printf debug_msg
#else
#define stream_printf(...)
#endif

typedef enum {
STREAM_STATE_CLOSED,
STREAM_STATE_OPEN,
Expand Down Expand Up @@ -75,9 +85,15 @@ static error_t open_hex(void *state);
static error_t write_hex(void *state, const uint8_t *data, uint32_t size);
static error_t close_hex(void *state);

stream_t stream[] = {
static bool detect_uhex_blocks(const uint8_t *data, uint32_t size);
static error_t open_uhex_blocks(void *state);
static error_t write_uhex_blocks(void *state, const uint8_t *data, uint32_t size);
static error_t close_uhex_blocks(void *state);

static stream_t stream[] = {
{detect_bin, open_bin, write_bin, close_bin}, // STREAM_TYPE_BIN
{detect_hex, open_hex, write_hex, close_hex}, // STREAM_TYPE_HEX
{detect_uhex_blocks, open_uhex_blocks, write_uhex_blocks, close_uhex_blocks}, // STREAM_TYPE_UHEX_BLOCKS
};
COMPILER_ASSERT(ARRAY_SIZE(stream) == STREAM_TYPE_COUNT);
// STREAM_TYPE_NONE must not be included in count
Expand All @@ -104,6 +120,7 @@ stream_type_t stream_start_identify(const uint8_t *data, uint32_t size)

for (i = STREAM_TYPE_START; i < STREAM_TYPE_COUNT; i++) {
if (stream[i].detect(data, size)) {
stream_printf("file_stream start_identify stream=%i\r\n", i);
return i;
}
}
Expand All @@ -118,12 +135,48 @@ stream_type_t stream_type_from_name(const vfs_filename_t filename)
if (0 == strncmp("BIN", &filename[8], 3)) {
return STREAM_TYPE_BIN;
} else if (0 == strncmp("HEX", &filename[8], 3)) {
return STREAM_TYPE_HEX;
return STREAM_TYPE_HEX_OR_UHEX;
} else {
return STREAM_TYPE_NONE;
}
}

bool stream_compatible(stream_type_t type_left, stream_type_t type_right)
{
if (type_left == type_right) {
return true;
}

if ((type_left == STREAM_TYPE_HEX_OR_UHEX &&
(type_right == STREAM_TYPE_HEX || type_right == STREAM_TYPE_UHEX_BLOCKS)) ||
(type_right == STREAM_TYPE_HEX_OR_UHEX &&
(type_left == STREAM_TYPE_HEX || type_left == STREAM_TYPE_UHEX_BLOCKS))) {
return true;
}

return false;
}

bool stream_self_contained_block(stream_type_t type, const uint8_t *data, uint32_t size)
{
switch (type) {
case STREAM_TYPE_BIN:
return false;

case STREAM_TYPE_HEX:
// A hex stream can also be a Universal Hex stream
return validate_uhex_block(data, size) ? true : false;

case STREAM_TYPE_UHEX_BLOCKS:
// The Universal Hex stream can be ordered (sectors) or unordered (blocks)
return validate_uhex_block(data, size) ? true : false;

default:
util_assert(0);
return false;
}
}

error_t stream_open(stream_type_t stream_type)
{
error_t status;
Expand All @@ -147,6 +200,7 @@ error_t stream_open(stream_type_t stream_type)
current_stream = &stream[stream_type];
// Initialize the specified stream
status = current_stream->open(&shared_state);
stream_printf("file_stream stream_open(type=%d); open ret=%d\r\n", stream_type, status);

if (ERROR_SUCCESS != status) {
state = STREAM_STATE_ERROR;
Expand All @@ -170,6 +224,7 @@ error_t stream_write(const uint8_t *data, uint32_t size)
stream_thread_assert();
// Write to stream
status = current_stream->write(&shared_state, data, size);
stream_printf("file_stream stream_write(size=%d); write ret=%d\r\n", size, status);

if (ERROR_SUCCESS_DONE == status) {
state = STREAM_STATE_END;
Expand Down Expand Up @@ -198,6 +253,7 @@ error_t stream_close(void)
stream_thread_assert();
// Close stream
status = current_stream->close(&shared_state);
stream_printf("file_stream stream_close; close ret=%d\r\n", status);
state = STREAM_STATE_CLOSED;
return status;
}
Expand Down Expand Up @@ -289,6 +345,13 @@ static error_t close_bin(void *state)

static bool detect_hex(const uint8_t *data, uint32_t size)
{
// Both Universal Hex formats will pass the normal hex file validation,
// but a Universal Hex in block format needs to be processed with the
// STREAM_TYPE_UHEX_BLOCKS stream.
// A Universal Hex in segment format can be be processed as a normal hex.
if (1 == validate_uhex_block(data, size)) {
return false;
}
return 1 == validate_hexfile(data);
}

Expand All @@ -315,6 +378,7 @@ static error_t write_hex(void *state, const uint8_t *data, uint32_t size)
while (1) {
// try to decode a block of hex data into bin data
parse_status = parse_hex_blob(data, size, &block_amt_parsed, hex_state->bin_buffer, sizeof(hex_state->bin_buffer), &bin_start_address, &bin_buf_written);
stream_printf("file_stream write_hex; parse_hex_blob ret=%d, bin_buf_written=%d\r\n", parse_status, bin_buf_written);

// the entire block of hex was decoded. This is a simple state
if (HEX_PARSE_OK == parse_status) {
Expand Down Expand Up @@ -364,3 +428,35 @@ static error_t close_hex(void *state)
status = flash_decoder_close();
return status;
}

/* Universal Hex, block format, file processing */
/* https://tech.microbit.org/software/spec-universal-hex/ */
/* The Universal Hex segment format is processed by the Intel Hex parser. */
/* This stream is for the Universal Hex block format only. */

static bool detect_uhex_blocks(const uint8_t *data, uint32_t size)
{
return 1 == validate_uhex_block(data, size);
}

static inline error_t open_uhex_blocks(void *state)
{
return open_hex(state);
}

static inline error_t write_uhex_blocks(void *state, const uint8_t *data, uint32_t size)
{
error_t status = write_hex(state, data, size);

// The block containing the EoF record could arrive at any point
if (ERROR_SUCCESS_DONE == status || ERROR_SUCCESS == status) {
status = ERROR_SUCCESS_DONE_OR_CONTINUE;
}

return status;
}

static inline error_t close_uhex_blocks(void *state)
{
return close_hex(state);
}
10 changes: 10 additions & 0 deletions source/daplink/drag-n-drop/file_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,15 @@ typedef enum {

STREAM_TYPE_BIN = STREAM_TYPE_START,
STREAM_TYPE_HEX,
STREAM_TYPE_UHEX_BLOCKS,

// Add new stream types here

STREAM_TYPE_COUNT,

// Needed as both formats share the same .hex extension
STREAM_TYPE_HEX_OR_UHEX,

STREAM_TYPE_NONE
} stream_type_t;

Expand All @@ -50,6 +54,12 @@ stream_type_t stream_start_identify(const uint8_t *data, uint32_t size);
// Stateless function to identify a filestream by its name
stream_type_t stream_type_from_name(const vfs_filename_t filename);

// Stateless function to identify if two streams are compatible
bool stream_compatible(stream_type_t type_left, stream_type_t type_right);

// Stateless function to identify if a stream can take blocks out of order
bool stream_self_contained_block(stream_type_t type, const uint8_t *data, uint32_t size);

error_t stream_open(stream_type_t stream_type);

error_t stream_write(const uint8_t *data, uint32_t size);
Expand Down
11 changes: 7 additions & 4 deletions source/daplink/drag-n-drop/flash_decoder.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ static bool flash_type_target_bin;

static bool flash_decoder_is_at_end(uint32_t addr, const uint8_t *data, uint32_t size);

__WEAK uint8_t board_detect_incompatible_image(const uint8_t *data, uint32_t size)
__WEAK uint8_t board_detect_incompatible_image(uint32_t addr, const uint8_t *data, uint32_t size)
{
return 0; // Return 0 if image is compatible
}
Expand Down Expand Up @@ -190,14 +190,14 @@ error_t flash_decoder_get_flash(flash_decoder_type_t type, uint32_t addr, bool a
return status;
}

error_t flash_decoder_validate_target_image(flash_decoder_type_t type, const uint8_t *data, uint32_t size)
error_t flash_decoder_validate_target_image(flash_decoder_type_t type, uint32_t addr, const uint8_t *data, uint32_t size)
{
error_t status = ERROR_SUCCESS;

if (daplink_is_interface()) {
if (FLASH_DECODER_TYPE_TARGET == type) {
if (g_board_info.target_cfg) {
if (board_detect_incompatible_image(data, size)){
if (board_detect_incompatible_image(addr, data, size)){
status = ERROR_FD_INCOMPATIBLE_IMAGE;
} else {
status = ERROR_SUCCESS;
Expand Down Expand Up @@ -295,7 +295,7 @@ error_t flash_decoder_write(uint32_t addr, const uint8_t *data, uint32_t size)

// Validate incompatible target image file
if (config_get_detect_incompatible_target()){
status = flash_decoder_validate_target_image(flash_type, flash_buf, flash_buf_pos);
status = flash_decoder_validate_target_image(flash_type, addr, flash_buf, flash_buf_pos);

if (ERROR_SUCCESS != status) {
state = DECODER_STATE_ERROR;
Expand Down Expand Up @@ -374,6 +374,9 @@ error_t flash_decoder_close(void)
if ((DECODER_STATE_DONE != prev_state) &&
(flash_type != FLASH_DECODER_TYPE_TARGET) &&
(status == ERROR_SUCCESS)) {
flash_decoder_printf(" error: update incomplete\r\n");
flash_decoder_printf(" prev_state=%d; flash_type=%d; status=%d\r\n",
prev_state, flash_type, status);
status = ERROR_IAP_UPDT_INCOMPLETE;
}

Expand Down
1 change: 0 additions & 1 deletion source/daplink/drag-n-drop/flash_decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ typedef enum {
flash_decoder_type_t flash_decoder_detect_type(const uint8_t *data, uint32_t size, uint32_t addr, bool addr_valid);
error_t flash_decoder_get_flash(flash_decoder_type_t type, uint32_t addr, bool addr_valid, uint32_t *start_addr, const flash_intf_t **flash_intf);

error_t flash_decoder_validate_target_image(flash_decoder_type_t type, const uint8_t *data, uint32_t size);
error_t flash_decoder_open(void);
error_t flash_decoder_write(uint32_t addr, const uint8_t *data, uint32_t size);
error_t flash_decoder_close(void);
Expand Down
48 changes: 29 additions & 19 deletions source/daplink/drag-n-drop/vfs_manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ typedef struct {
bool stream_started; // Stream processing started. This only gets reset remount
bool stream_finished; // Stream processing is done. This only gets reset remount
bool stream_optional_finish; // True if the stream processing can be considered done
bool stream_ooo_blocks; // True if the stream data blocks can be processed out of order
bool file_info_optional_finish; // True if the file transfer can be considered done
bool transfer_timeout; // Set if the transfer was finished because of a timeout. This only gets reset remount
stream_type_t stream; // Current stream or STREAM_TYPE_NONE is stream is closed. This only gets reset remount
Expand Down Expand Up @@ -112,6 +113,7 @@ static const file_transfer_state_t default_transfer_state = {
false,
false,
false,
false,
STREAM_TYPE_NONE,
};

Expand Down Expand Up @@ -439,27 +441,39 @@ static void file_change_handler(const vfs_filename_t filename, vfs_file_change_t
static void file_data_handler(uint32_t sector, const uint8_t *buf, uint32_t num_of_sectors)
{
stream_type_t stream;
uint32_t size;
uint32_t size = VFS_SECTOR_SIZE * num_of_sectors;

// this is the key for starting a file write - we dont care what file types are sent
// just look for something unique (NVIC table, hex, srec, etc) until root dir is updated
if (!file_transfer_state.stream_started) {
// look for file types we can program
stream = stream_start_identify((uint8_t *)buf, VFS_SECTOR_SIZE * num_of_sectors);
stream = stream_start_identify((uint8_t *)buf, size);

if (STREAM_TYPE_NONE != stream) {
transfer_stream_open(stream, sector);
}
}

if (file_transfer_state.stream_started) {
// Ignore sectors coming before this file
if (sector < file_transfer_state.start_sector) {
return;
}
bool self_contained_block = stream_self_contained_block(file_transfer_state.stream, buf, size);

// sectors must be in order
if (sector != file_transfer_state.file_next_sector) {
if (self_contained_block) {
vfs_mngr_printf("vfs_manager file_data_handler sector=%i\r\n", sector);
vfs_mngr_printf(" sector self-contained\r\n");
file_transfer_state.stream_ooo_blocks = true;
} else if (file_transfer_state.stream_ooo_blocks) {
// Invalid block for a self-contained stream, can be ignored
vfs_mngr_printf("vfs_manager file_data_handler sector=%i\r\n", sector);
vfs_mngr_printf(" invalid data for self-contained stream\r\n");
vfs_mngr_printf(" size=%d data=%x,%x,%x,%x,...,%x,%x,%x,%x\r\n",
size, buf[0], buf[1], buf[2], buf[3], buf[size - 4],
buf[size - 3], buf[size - 2], buf[size - 1]);
return;
} else if (sector < file_transfer_state.start_sector) {
// For ordered streams, ignore sectors coming before this file
return;
} else if (sector != file_transfer_state.file_next_sector) {
// For ordered streams, sector must be the next in the sequence
vfs_mngr_printf("vfs_manager file_data_handler sector=%i\r\n", sector);

if (sector < file_transfer_state.file_next_sector) {
Expand All @@ -482,7 +496,6 @@ static void file_data_handler(uint32_t sector, const uint8_t *buf, uint32_t num_
}

// This sector could be part of the file so record it
size = VFS_SECTOR_SIZE * num_of_sectors;
file_transfer_state.size_transferred += size;
file_transfer_state.file_next_sector = sector + num_of_sectors;

Expand Down Expand Up @@ -584,15 +597,6 @@ static void transfer_update_file_info(vfs_file_t file, uint32_t start_sector, ui
}
}

// Initialize the stream if it has not been set
if (STREAM_TYPE_NONE == file_transfer_state.stream) {
file_transfer_state.stream = stream;

if (stream != STREAM_TYPE_NONE) {
vfs_mngr_printf(" stream=%i\r\n", stream);
}
}

// Check - File size must either grow or be smaller than the size already transferred
if ((size < file_transfer_state.file_size) && (size < file_transfer_state.size_transferred) && (size > 0)) {
vfs_mngr_printf(" error: file size changed from %i to %i\r\n", file_transfer_state.file_size, size);
Expand All @@ -608,7 +612,7 @@ static void transfer_update_file_info(vfs_file_t file, uint32_t start_sector, ui
}

// Check - stream must be the same
if ((stream != STREAM_TYPE_NONE) && (stream != file_transfer_state.stream)) {
if (stream != STREAM_TYPE_NONE && file_transfer_state.stream != STREAM_TYPE_NONE && !stream_compatible(file_transfer_state.stream, stream)) {
vfs_mngr_printf(" error: changed types during transfer from %i to %i\r\n", file_transfer_state.stream, stream);
transfer_update_state(ERROR_ERROR_DURING_TRANSFER);
return;
Expand Down Expand Up @@ -761,6 +765,12 @@ static void transfer_update_state(error_t status)
(file_transfer_state.size_transferred >= file_transfer_state.file_size) &&
(file_transfer_state.file_size > 0) &&
(file_transfer_state.start_sector == file_transfer_state.file_start_sector);
if (file_transfer_state.stream_ooo_blocks &&
file_transfer_state.file_size > 0 &&
file_transfer_state.size_transferred >= file_transfer_state.file_size) {
vfs_mngr_printf(" OoO file_info_optional_finish=%d\r\n", file_transfer_state.file_info_optional_finish);
file_transfer_state.file_info_optional_finish = true;
}
transfer_timeout = file_transfer_state.transfer_timeout;
transfer_started = (VFS_FILE_INVALID != file_transfer_state.file_to_program) ||
(STREAM_TYPE_NONE != file_transfer_state.stream);
Expand Down
9 changes: 9 additions & 0 deletions source/daplink/validation.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,12 @@ uint8_t validate_hexfile(const uint8_t *buf)
return ((buf[0] == ':') && ((buf[8] == '0') || (buf[8] == '2') || (buf[8] == '3') || (buf[8] == '4') || (buf[8] == '5'))) ? 1 : 0;
}
}

uint8_t validate_uhex_block(const uint8_t *buf, uint32_t size) {
if (size != 512) {
return 0;
}
return (memcmp(buf, (const void *)":02000004", 9) == 0) &&
(memcmp(buf + 16, (const void *)":0400000A", 9) == 0) &&
(buf[511] == '\n');
}
Loading