From 5e93cca0a9c2a7a017b827ea82335b44a662c15c Mon Sep 17 00:00:00 2001 From: Pavol Rusnak Date: Sat, 16 Dec 2017 17:54:04 +0100 Subject: [PATCH] WIP --- embed/boardloader/main.c | 14 ++++++++------ embed/bootloader/messages.c | 10 +++++----- embed/prodtest/main.c | 12 ++++++------ embed/trezorhal/flash.c | 11 ++++++----- embed/trezorhal/flash.h | 24 ++++++++++++------------ embed/trezorhal/image.h | 8 ++++---- embed/trezorhal/sdcard.c | 5 ++--- embed/trezorhal/sdcard.h | 10 +++++----- embed/trezorhal/secbool.h | 4 ++++ embed/trezorhal/usb.c | 2 +- embed/trezorhal/usb_hid-defs.h | 16 ++++++++-------- embed/trezorhal/usb_vcp-defs.h | 14 +++++++------- embed/unix/sdcard.c | 3 +-- 13 files changed, 69 insertions(+), 64 deletions(-) diff --git a/embed/boardloader/main.c b/embed/boardloader/main.c index d1fbb42a7..76aaa5ad4 100644 --- a/embed/boardloader/main.c +++ b/embed/boardloader/main.c @@ -30,7 +30,7 @@ static uint32_t check_sdcard(void) return 0; } - sdcard_power_on(); + ensure(sdcard_power_on(), NULL); uint64_t cap = sdcard_get_capacity_in_bytes(); if (cap < 1024 * 1024) { @@ -122,23 +122,23 @@ static secbool copy_sdcard(void) // copy bootloader from SD card to Flash display_printf("copying new bootloader from SD card\n\n"); - sdcard_power_on(); + ensure(sdcard_power_on(), NULL); uint32_t buf[SDCARD_BLOCK_SIZE / sizeof(uint32_t)]; for (int i = 0; i < (IMAGE_HEADER_SIZE + codelen) / SDCARD_BLOCK_SIZE; i++) { - sdcard_read_blocks(buf, i, 1); + ensure(sdcard_read_blocks(buf, i, 1), NULL); for (int j = 0; j < SDCARD_BLOCK_SIZE / sizeof(uint32_t); j++) { if (sectrue != flash_write_word(BOOTLOADER_START + i * SDCARD_BLOCK_SIZE + j * sizeof(uint32_t), buf[j])) { display_printf("copy failed\n"); sdcard_power_off(); - flash_lock(); + ensure(flash_lock(), NULL); return secfalse; } } } sdcard_power_off(); - flash_lock(); + ensure(flash_lock(), NULL); display_printf("\ndone\n\n"); display_printf("Unplug the device and remove the SD card\n"); @@ -161,7 +161,9 @@ int main(void) FLASH_SECTOR_STORAGE_1, FLASH_SECTOR_STORAGE_2, }; - flash_erase_sectors(sectors, sizeof(sectors), NULL); + // display is not initialized so don't call ensure + secbool r = flash_erase_sectors(sectors, sizeof(sectors), NULL); + (void)r; return 2; } diff --git a/embed/bootloader/messages.c b/embed/bootloader/messages.c index 46d3be567..11695738b 100644 --- a/embed/bootloader/messages.c +++ b/embed/bootloader/messages.c @@ -58,7 +58,7 @@ static bool _usb_write(pb_ostream_t *stream, const pb_byte_t *buf, size_t count) memcpy(state->buf + state->packet_pos, buf + written, USB_PACKET_SIZE - state->packet_pos); written += USB_PACKET_SIZE - state->packet_pos; // send packet - usb_hid_write_blocking(state->iface_num, state->buf, USB_PACKET_SIZE, 100); + ensure(usb_hid_write_blocking(state->iface_num, state->buf, USB_PACKET_SIZE, 100), NULL); // prepare new packet state->packet_index++; memset(state->buf, 0, USB_PACKET_SIZE); @@ -78,7 +78,7 @@ static void _usb_write_flush(usb_write_state *state) memset(state->buf + state->packet_pos, 0, USB_PACKET_SIZE - state->packet_pos); } // send packet - usb_hid_write_blocking(state->iface_num, state->buf, USB_PACKET_SIZE, 100); + ensure(usb_hid_write_blocking(state->iface_num, state->buf, USB_PACKET_SIZE, 100), NULL); } static secbool _send_msg(uint8_t iface_num, uint16_t msg_id, const pb_field_t fields[], const void *msg) @@ -157,7 +157,7 @@ static bool _usb_read(pb_istream_t *stream, uint8_t *buf, size_t count) memcpy(buf + read, state->buf + state->packet_pos, USB_PACKET_SIZE - state->packet_pos); read += USB_PACKET_SIZE - state->packet_pos; // read next packet - usb_hid_read_blocking(state->iface_num, state->buf, USB_PACKET_SIZE, 100); + ensure(usb_hid_read_blocking(state->iface_num, state->buf, USB_PACKET_SIZE, 100), NULL); // prepare next packet state->packet_index++; state->packet_pos = MSG_HEADER2_LEN; @@ -427,12 +427,12 @@ int process_msg_FirmwareUpload(uint8_t iface_num, uint32_t msg_size, uint8_t *bu MSG_SEND_ASSIGN_VALUE(code, FailureType_Failure_ProcessError); MSG_SEND_ASSIGN_STRING(message, "Could not write data"); MSG_SEND(Failure); - flash_lock(); + ensure(flash_lock(), NULL); return -6; } } - flash_lock(); + ensure(flash_lock(), NULL); firmware_remaining -= chunk_requested; firmware_block++; diff --git a/embed/prodtest/main.c b/embed/prodtest/main.c index 1464ac0d5..a548179b6 100644 --- a/embed/prodtest/main.c +++ b/embed/prodtest/main.c @@ -32,13 +32,13 @@ static void vcp_intr(void) static void vcp_puts(const char *s, size_t len) { - usb_vcp_write_blocking(VCP_IFACE, (const uint8_t *) s, len, -1); + ensure(usb_vcp_write_blocking(VCP_IFACE, (const uint8_t *) s, len, -1), NULL); } static char vcp_getchar(void) { uint8_t c = 0; - usb_vcp_read_blocking(VCP_IFACE, &c, 1, -1); + ensure(usb_vcp_read_blocking(VCP_IFACE, &c, 1, -1), NULL); return (char) c; } @@ -221,7 +221,7 @@ static void test_sd(void) return; } - sdcard_power_on(); + ensure(sdcard_power_on(), NULL); if (sectrue != sdcard_read_blocks(buf1, 0, BLOCK_SIZE / SDCARD_BLOCK_SIZE)) { vcp_printf("ERROR sdcard_read_blocks (0)"); goto power_off; @@ -261,7 +261,7 @@ static void test_otp_read(void) { uint8_t data[32]; memset(data, 0, sizeof(data)); - flash_otp_read(0, 0, data, sizeof(data)); + ensure(flash_otp_read(0, 0, data, sizeof(data)), NULL); // strip trailing 0xFF for (size_t i = 0; i < sizeof(data); i++) { @@ -284,8 +284,8 @@ static void test_otp_write(const char *args) char data[32]; memset(data, 0, sizeof(data)); strncpy(data, args, sizeof(data) - 1); - flash_otp_write(0, 0, (const uint8_t *) data, sizeof(data)); - flash_otp_lock(0); + ensure(flash_otp_write(0, 0, (const uint8_t *) data, sizeof(data)), NULL); + ensure(flash_otp_lock(0), NULL); vcp_printf("OK"); } diff --git a/embed/trezorhal/flash.c b/embed/trezorhal/flash.c index a6c3bfb22..fab664017 100644 --- a/embed/trezorhal/flash.c +++ b/embed/trezorhal/flash.c @@ -9,6 +9,7 @@ #include +#include "common.h" #include "flash.h" // see docs/memory.md for more information @@ -87,14 +88,14 @@ secbool flash_erase_sectors(const uint8_t *sectors, int len, void (*progress)(in EraseInitStruct.Sector = sectors[i]; uint32_t SectorError; if (HAL_FLASHEx_Erase(&EraseInitStruct, &SectorError) != HAL_OK) { - flash_lock(); + ensure(flash_lock(), NULL); return secfalse; } // check whether the sector was really deleted (contains only 0xFF) const uint32_t addr_start = FLASH_SECTOR_TABLE[sectors[i]], addr_end = FLASH_SECTOR_TABLE[sectors[i] + 1]; for (uint32_t addr = addr_start; addr < addr_end; addr += 4) { if (*((const uint32_t *)addr) != 0xFFFFFFFF) { - flash_lock(); + ensure(flash_lock(), NULL); return secfalse; } } @@ -102,7 +103,7 @@ secbool flash_erase_sectors(const uint8_t *sectors, int len, void (*progress)(in progress(i + 1, len); } } - flash_lock(); + ensure(flash_lock(), NULL); return sectrue; } @@ -166,7 +167,7 @@ secbool flash_otp_write(uint8_t block, uint8_t offset, const uint8_t *data, uint break; } } - flash_lock(); + ensure(flash_lock(), NULL); return ret; } @@ -179,7 +180,7 @@ secbool flash_otp_lock(uint8_t block) return secfalse; } HAL_StatusTypeDef ret = HAL_FLASH_Program(FLASH_TYPEPROGRAM_BYTE, FLASH_OTP_LOCK_BASE + block, 0x00); - flash_lock(); + ensure(flash_lock(), NULL); return sectrue * (ret == HAL_OK); } diff --git a/embed/trezorhal/flash.h b/embed/trezorhal/flash.h index 9fc9c4ddc..380f0c760 100644 --- a/embed/trezorhal/flash.h +++ b/embed/trezorhal/flash.h @@ -45,25 +45,25 @@ void flash_init(void); -secbool flash_unlock(void); -secbool flash_lock(void); +secbool __wur flash_unlock(void); +secbool __wur flash_lock(void); const void *flash_get_address(uint8_t sector, uint32_t offset, uint32_t size); -secbool flash_erase_sectors(const uint8_t *sectors, int len, void (*progress)(int pos, int len)); +secbool __wur flash_erase_sectors(const uint8_t *sectors, int len, void (*progress)(int pos, int len)); static inline secbool flash_erase_sector(uint8_t sector) { return flash_erase_sectors(§or, 1, NULL); } -secbool flash_write_byte(uint32_t address, uint8_t data); -secbool flash_write_word(uint32_t address, uint32_t data); -secbool flash_write_byte_rel(uint8_t sector, uint32_t offset, uint8_t data); -secbool flash_write_word_rel(uint8_t sector, uint32_t offset, uint32_t data); -secbool flash_read_word_rel(uint8_t sector, uint32_t offset, uint32_t *data); +secbool __wur flash_write_byte(uint32_t address, uint8_t data); +secbool __wur flash_write_word(uint32_t address, uint32_t data); +secbool __wur flash_write_byte_rel(uint8_t sector, uint32_t offset, uint8_t data); +secbool __wur flash_write_word_rel(uint8_t sector, uint32_t offset, uint32_t data); +secbool __wur flash_read_word_rel(uint8_t sector, uint32_t offset, uint32_t *data); #define FLASH_OTP_NUM_BLOCKS 16 #define FLASH_OTP_BLOCK_SIZE 32 -secbool flash_otp_read(uint8_t block, uint8_t offset, uint8_t *data, uint8_t datalen); -secbool flash_otp_write(uint8_t block, uint8_t offset, const uint8_t *data, uint8_t datalen); -secbool flash_otp_lock(uint8_t block); -secbool flash_otp_is_locked(uint8_t block); +secbool __wur flash_otp_read(uint8_t block, uint8_t offset, uint8_t *data, uint8_t datalen); +secbool __wur flash_otp_write(uint8_t block, uint8_t offset, const uint8_t *data, uint8_t datalen); +secbool __wur flash_otp_lock(uint8_t block); +secbool __wur flash_otp_is_locked(uint8_t block); #endif // TREZORHAL_FLASH_H diff --git a/embed/trezorhal/image.h b/embed/trezorhal/image.h index ca37e85cd..646143176 100644 --- a/embed/trezorhal/image.h +++ b/embed/trezorhal/image.h @@ -59,14 +59,14 @@ typedef struct { uint8_t sig[64]; } vendor_header; -secbool load_image_header(const uint8_t * const data, const uint32_t magic, const uint32_t maxsize, uint8_t key_m, uint8_t key_n, const uint8_t * const *keys, image_header * const hdr); +secbool __wur load_image_header(const uint8_t * const data, const uint32_t magic, const uint32_t maxsize, uint8_t key_m, uint8_t key_n, const uint8_t * const *keys, image_header * const hdr); -secbool load_vendor_header(const uint8_t * const data, uint8_t key_m, uint8_t key_n, const uint8_t * const *keys, vendor_header * const vhdr); +secbool __wur load_vendor_header(const uint8_t * const data, uint8_t key_m, uint8_t key_n, const uint8_t * const *keys, vendor_header * const vhdr); void vendor_keys_hash(const vendor_header * const vhdr, uint8_t *hash); -secbool check_single_hash(const uint8_t * const hash, const uint8_t * const data, int len); +secbool __wur check_single_hash(const uint8_t * const hash, const uint8_t * const data, int len); -secbool check_image_contents(const image_header * const hdr, uint32_t firstskip, const uint8_t *sectors, int blocks); +secbool __wur check_image_contents(const image_header * const hdr, uint32_t firstskip, const uint8_t *sectors, int blocks); #endif diff --git a/embed/trezorhal/sdcard.c b/embed/trezorhal/sdcard.c index c4606a63f..fc022b102 100644 --- a/embed/trezorhal/sdcard.c +++ b/embed/trezorhal/sdcard.c @@ -108,13 +108,12 @@ error: return secfalse; } -secbool sdcard_power_off(void) { +void sdcard_power_off(void) { if (NULL == sd_handle.Instance) { - return sectrue; + return; } HAL_SD_DeInit(&sd_handle); sd_handle.Instance = NULL; - return sectrue; } uint64_t sdcard_get_capacity_in_bytes(void) { diff --git a/embed/trezorhal/sdcard.h b/embed/trezorhal/sdcard.h index 011180f78..39547b0cf 100644 --- a/embed/trezorhal/sdcard.h +++ b/embed/trezorhal/sdcard.h @@ -33,11 +33,11 @@ #define SDCARD_BLOCK_SIZE (512) void sdcard_init(void); -secbool sdcard_is_present(void); -secbool sdcard_power_on(void); -secbool sdcard_power_off(void); +secbool __wur sdcard_is_present(void); +secbool __wur sdcard_power_on(void); +void sdcard_power_off(void); uint64_t sdcard_get_capacity_in_bytes(void); -secbool sdcard_read_blocks(uint32_t *dest, uint32_t block_num, uint32_t num_blocks); -secbool sdcard_write_blocks(const uint32_t *src, uint32_t block_num, uint32_t num_blocks); +secbool __wur sdcard_read_blocks(uint32_t *dest, uint32_t block_num, uint32_t num_blocks); +secbool __wur sdcard_write_blocks(const uint32_t *src, uint32_t block_num, uint32_t num_blocks); #endif diff --git a/embed/trezorhal/secbool.h b/embed/trezorhal/secbool.h index b4485217c..ab6e8da34 100644 --- a/embed/trezorhal/secbool.h +++ b/embed/trezorhal/secbool.h @@ -7,4 +7,8 @@ typedef uint32_t secbool; #define sectrue 0xAAAAAAAAU #define secfalse 0x00000000U +#ifndef __wur +#define __wur __attribute__ ((warn_unused_result)) +#endif + #endif diff --git a/embed/trezorhal/usb.c b/embed/trezorhal/usb.c index f055aae48..242dfe35f 100644 --- a/embed/trezorhal/usb.c +++ b/embed/trezorhal/usb.c @@ -42,7 +42,7 @@ static USBD_HandleTypeDef usb_dev_handle; static const USBD_DescriptorsTypeDef usb_descriptors; static const USBD_ClassTypeDef usb_class; -static secbool check_desc_str(const uint8_t *s) { +static secbool __wur check_desc_str(const uint8_t *s) { if (NULL == s) return secfalse; if (strlen((const char *)s) > USB_MAX_STR_SIZE) return secfalse; return sectrue; diff --git a/embed/trezorhal/usb_hid-defs.h b/embed/trezorhal/usb_hid-defs.h index 8ffcc1747..5597e0328 100644 --- a/embed/trezorhal/usb_hid-defs.h +++ b/embed/trezorhal/usb_hid-defs.h @@ -69,12 +69,12 @@ typedef struct { uint8_t ep_in_is_idle; // Set to 1 after IN endpoint gets idle } usb_hid_state_t; -secbool usb_hid_add(const usb_hid_info_t *hid_info); -secbool usb_hid_can_read(uint8_t iface_num); -secbool usb_hid_can_write(uint8_t iface_num); -int usb_hid_read(uint8_t iface_num, uint8_t *buf, uint32_t len); -int usb_hid_write(uint8_t iface_num, const uint8_t *buf, uint32_t len); +secbool __wur usb_hid_add(const usb_hid_info_t *hid_info); +secbool __wur usb_hid_can_read(uint8_t iface_num); +secbool __wur usb_hid_can_write(uint8_t iface_num); +int __wur usb_hid_read(uint8_t iface_num, uint8_t *buf, uint32_t len); +int __wur usb_hid_write(uint8_t iface_num, const uint8_t *buf, uint32_t len); -int usb_hid_read_select(uint32_t timeout); -int usb_hid_read_blocking(uint8_t iface_num, uint8_t *buf, uint32_t len, int timeout); -int usb_hid_write_blocking(uint8_t iface_num, const uint8_t *buf, uint32_t len, int timeout); +int __wur usb_hid_read_select(uint32_t timeout); +int __wur usb_hid_read_blocking(uint8_t iface_num, uint8_t *buf, uint32_t len, int timeout); +int __wur usb_hid_write_blocking(uint8_t iface_num, const uint8_t *buf, uint32_t len, int timeout); diff --git a/embed/trezorhal/usb_vcp-defs.h b/embed/trezorhal/usb_vcp-defs.h index 7b36cb7b8..23fb7edfa 100644 --- a/embed/trezorhal/usb_vcp-defs.h +++ b/embed/trezorhal/usb_vcp-defs.h @@ -119,11 +119,11 @@ typedef struct { uint8_t ep_in_is_idle; // Set to 1 after IN endpoint gets idle } usb_vcp_state_t; -secbool usb_vcp_add(const usb_vcp_info_t *vcp_info); -secbool usb_vcp_can_read(uint8_t iface_num); -secbool usb_vcp_can_write(uint8_t iface_num); -int usb_vcp_read(uint8_t iface_num, uint8_t *buf, uint32_t len); -int usb_vcp_write(uint8_t iface_num, const uint8_t *buf, uint32_t len); +secbool __wur usb_vcp_add(const usb_vcp_info_t *vcp_info); +secbool __wur usb_vcp_can_read(uint8_t iface_num); +secbool __wur usb_vcp_can_write(uint8_t iface_num); +int __wur usb_vcp_read(uint8_t iface_num, uint8_t *buf, uint32_t len); +int __wur usb_vcp_write(uint8_t iface_num, const uint8_t *buf, uint32_t len); -int usb_vcp_read_blocking(uint8_t iface_num, uint8_t *buf, uint32_t len, int timeout); -int usb_vcp_write_blocking(uint8_t iface_num, const uint8_t *buf, uint32_t len, int timeout); +int __wur usb_vcp_read_blocking(uint8_t iface_num, uint8_t *buf, uint32_t len, int timeout); +int __wur usb_vcp_write_blocking(uint8_t iface_num, const uint8_t *buf, uint32_t len, int timeout); diff --git a/embed/unix/sdcard.c b/embed/unix/sdcard.c index 1f48a861f..db670c42c 100644 --- a/embed/unix/sdcard.c +++ b/embed/unix/sdcard.c @@ -73,9 +73,8 @@ secbool sdcard_power_on(void) { return sectrue; } -secbool sdcard_power_off(void) { +void sdcard_power_off(void) { sdcard_powered = secfalse; - return sectrue; } uint64_t sdcard_get_capacity_in_bytes(void) {