From 79c0cf67149d02981dc13cca2f3a4bec1e656f63 Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Mon, 9 Oct 2023 18:22:54 +0200 Subject: [PATCH 01/13] Fix conflict name issue during APP building - Separate object files into 2 distinct directories for both SDK and APP - Same modification for dependency files - Warn if same headers filename are found in both SDK and APP - Simplify the way source files list is generated (cherry picked from commit cc8702b057c82a08424286567bf39a9387f28353) --- Makefile.rules | 60 +++++++++++++++++++++++++++++++++++++----- Makefile.rules_generic | 36 +++++++++++++++++++++++-- 2 files changed, 88 insertions(+), 8 deletions(-) diff --git a/Makefile.rules b/Makefile.rules index 8769969dc..565c115b9 100644 --- a/Makefile.rules +++ b/Makefile.rules @@ -24,6 +24,17 @@ ifeq ($(DISABLE_UI),0) endif endif +define uniq = + $(eval seen :=) + $(foreach _,$1,$(if $(filter $_,${seen}),,$(eval seen += $_))) + ${seen} +endef + +define check_duplicate = + $(eval LIST := $(sort $(foreach file_h, $(notdir $1), $(notdir $(shell find $2 -name $(file_h)))))) + $(if $(LIST), $(info [WARNING] Found duplicate files in SDK and APP: ${LIST})) +endef + # adding the correct target header to sources SDK_SOURCE_PATH += target/$(TARGET)/include @@ -35,12 +46,49 @@ ifeq ($(NO_CXNG),) INCLUDES_PATH += $(BOLOS_SDK)/lib_cxng/include endif -SOURCE_PATH += $(BOLOS_SDK)/src $(foreach libdir, $(SDK_SOURCE_PATH), $(dir $(shell find $(BOLOS_SDK)/$(libdir) -name '*.[csS]'))) $(dir $(shell find $(APP_SOURCE_PATH) -name '*.[csS]')) -SOURCE_FILES += $(shell find $(SOURCE_PATH) -name '*.[csS]') $(GLYPH_DESTC) $(APP_SOURCE_FILES) -INCLUDES_PATH += $(dir $(foreach libdir, $(SDK_SOURCE_PATH), $(dir $(shell find $(BOLOS_SDK)/$(libdir) -name '*.h')))) include $(BOLOS_SDK)/include $(BOLOS_SDK)/include/arm $(dir $(shell find $(APP_SOURCE_PATH) -name '*.h')) $(GLYPH_SRC_DIR) +# Get absolute App root directory to ensure correct stem replacement +APP_DIR := $(shell git rev-parse --show-toplevel) +ifeq ($(APP_DIR),) +MSG := "[ERROR] You should be inside a git repo (you can use 'git init')" +ifneq ($(ENABLE_SDK_WERROR),0) +$(error $(MSG)) +else +$(warning $(MSG)) +endif +endif + +# Extract BOLOS_SDK source files added by the App Makefile +APP_SRC_FROM_SDK = $(filter $(BOLOS_SDK)/%, $(APP_SOURCE_FILES)) +# Extract generated and glyphs source files added by the App Makefile +APP_SRC_GEN = $(filter $(GEN_SRC_DIR)/%, $(APP_SOURCE_FILES)) +# Extract proto source files added by the App Makefile +APP_SRC_PROTOC = $(filter %.pb.c, $(APP_SOURCE_FILES) $(SOURCE_FILES)) $(shell find $(APP_SOURCE_PATH) -name '*.pb.c') +# Filter remaining real source files from App (proto filtered globally hereafter) +APP_SRC_FILTER = $(filter-out $(APP_SRC_FROM_SDK) $(APP_SRC_GEN), $(APP_SOURCE_FILES)) + +# Separate SDK and APP and GEN sources +SOURCES_SDK += $(foreach libdir, src $(SDK_SOURCE_PATH), $(shell find $(BOLOS_SDK)/$(libdir) -name '*.[csS]')) $(APP_SRC_FROM_SDK) +SOURCES_APP += $(filter-out %.pb.c, $(abspath $(SOURCE_FILES) $(foreach libdir, $(SOURCE_PATH) $(APP_SOURCE_PATH), $(shell find $(libdir) -name '*.[csS]')) $(APP_SRC_FILTER))) +SOURCES_GEN += $(GLYPH_DESTC) $(APP_SRC_GEN) +VPATH += $(call uniq, $(dir $(SOURCES_SDK) $(SOURCES_APP) $(GLYPH_DESTC))) + +# Retrieve APP header filenames +INCLUDES_APP += $(shell find $(APP_SOURCE_PATH) -name '*.h') +# Warn if a same header filename is found in both APP and SDK +$(call check_duplicate, $(INCLUDES_APP), $(BOLOS_SDK)) +# Compute header directories list +INCLUDES_PATH += $(call uniq, $(dir $(foreach libdir, $(SDK_SOURCE_PATH), $(dir $(shell find $(BOLOS_SDK)/$(libdir) -name '*.h')))) include $(BOLOS_SDK)/include $(BOLOS_SDK)/include/arm $(dir $(INCLUDES_APP)) $(GLYPH_SRC_DIR)) + +# Separate object files from SDK and APP to avoid name conflicts +OBJECTS_SDK += $(sort $(subst $(BOLOS_SDK), $(OBJ_DIR)/sdk, $(addsuffix .o, $(basename $(SOURCES_SDK))))) +OBJECTS_APP += $(sort $(addprefix $(OBJ_DIR)/app, $(addsuffix .o, $(basename $(foreach f, $(SOURCES_APP), $(shell echo $(f) | sed "s|$(APP_DIR)||" 2>/dev/null)))))) +OBJECTS_GEN += $(sort $(SOURCES_GEN:%c=%o)) +OBJECTS_PROTOC += $(sort $(addprefix $(OBJ_DIR)/, $(addsuffix .o, $(basename $(APP_SRC_PROTOC))))) +OBJECT_FILES = $(OBJECTS_GEN) $(OBJECTS_PROTOC) $(OBJECTS_APP) $(OBJECTS_SDK) +OBJECTS_DIR += $(sort $(dir $(OBJECT_FILES))) -VPATH += $(dir $(SOURCE_FILES)) -OBJECT_FILES += $(sort $(addprefix $(OBJ_DIR)/, $(addsuffix .o, $(basename $(notdir $(SOURCE_FILES)))))) -DEPEND_FILES += $(sort $(addprefix $(DEP_DIR)/, $(addsuffix .d, $(basename $(notdir $(SOURCE_FILES)))))) +# Separate dependency files from SDK and APP to avoid name conflicts +DEPEND_FILES = $(subst $(OBJ_DIR), $(DEP_DIR), $(addsuffix .d, $(basename $(OBJECT_FILES)))) +DEPEND_DIR += $(sort $(dir $(DEPEND_FILES))) include $(BOLOS_SDK)/Makefile.rules_generic diff --git a/Makefile.rules_generic b/Makefile.rules_generic index 584fe343e..a3170dccd 100644 --- a/Makefile.rules_generic +++ b/Makefile.rules_generic @@ -71,7 +71,7 @@ BUILD_DEPENDENCIES += $(APP_CUSTOM_BUILD_DEPENDENCIES) prepare: $(L)echo Prepare directories - @mkdir -p $(BIN_DIR) $(OBJ_DIR) $(DBG_DIR) $(DEP_DIR) $(GEN_SRC_DIR) bin debug + @mkdir -p $(BIN_DIR) $(OBJ_DIR) $(OBJECTS_DIR) $(DBG_DIR) $(DEP_DIR) $(DEPEND_DIR) $(GEN_SRC_DIR) bin debug $(BUILD_DEPENDENCIES): prepare @@ -80,6 +80,38 @@ DBG_TARGETS := debug/app.map debug/app.asm default: $(BIN_TARGETS) $(DBG_TARGETS) +# Glyphs target +$(GEN_SRC_DIR)/%.o: $(GEN_SRC_DIR)/%.c $(BUILD_DEPENDENCIES) prepare + @echo "[CC] $@" + $(L)$(call cc_cmdline,$(INCLUDES_PATH), $(DEFINES),$<,$@) + +# App files targets +$(OBJ_DIR)/app/%.o: $(APP_DIR)/%.c $(BUILD_DEPENDENCIES) prepare + @echo "[CC] $@" + $(L)$(call cc_cmdline,$(INCLUDES_PATH), $(DEFINES),$<,$@) + +$(OBJ_DIR)/app/%.o: $(APP_DIR)/%.s $(BUILD_DEPENDENCIES) prepare + @echo "[AS] $@" + $(L)$(call as_cmdline,$(INCLUDES_PATH), $(DEFINES),$<,$@) + +$(OBJ_DIR)/app/%.o: $(APP_DIR)/%.S $(BUILD_DEPENDENCIES) prepare + @echo "[AS] $@" + $(L)$(call as_cmdline,$(INCLUDES_PATH), $(DEFINES),$<,$@) + +# SDK files targets +$(OBJ_DIR)/sdk/%.o: $(BOLOS_SDK)/%.c $(BUILD_DEPENDENCIES) prepare + @echo "[CC] $@" + $(L)$(call cc_cmdline,$(INCLUDES_PATH), $(DEFINES),$<,$@) + +$(OBJ_DIR)/sdk/%.o: $(BOLOS_SDK)/%.s $(BUILD_DEPENDENCIES) prepare + @echo "[AS] $@" + $(L)$(call as_cmdline,$(INCLUDES_PATH), $(DEFINES),$<,$@) + +$(OBJ_DIR)/sdk/%.o: $(BOLOS_SDK)/%.S $(BUILD_DEPENDENCIES) prepare + @echo "[AS] $@" + $(L)$(call as_cmdline,$(INCLUDES_PATH), $(DEFINES),$<,$@) + +# Generic targets $(OBJ_DIR)/%.o: %.c $(BUILD_DEPENDENCIES) prepare @echo "[CC] $@" $(L)$(call cc_cmdline,$(INCLUDES_PATH), $(DEFINES),$<,$@) @@ -163,7 +195,7 @@ endif # cc_cmdline(include,defines,src,dest) Macro that is used to format arguments for the compiler # dependency files are generated along the object file -cc_cmdline = $(CC) -c $(CFLAGS) -MMD -MT $(OBJ_DIR)/$(basename $(notdir $(4))).o -MF $(DEP_DIR)/$(basename $(notdir $(4))).d $(addprefix -D,$(2)) $(addprefix -I,$(1)) -o $(4) $(3) +cc_cmdline = $(CC) -c $(CFLAGS) -MMD -MT $(4) -MF $(subst $(OBJ_DIR), $(DEP_DIR), $(addsuffix .d, $(basename $(4)))) $(addprefix -D,$(2)) $(addprefix -I,$(1)) -o $(4) $(3) as_cmdline = $(AS) -c $(AFLAGS) $(addprefix -D,$(2)) $(addprefix -I,$(1)) -o $(4) $(3) From 3d16251986ee08f2a9c56d0b504853651397060a Mon Sep 17 00:00:00 2001 From: Xavier Chapron Date: Wed, 10 Jan 2024 11:56:39 +0100 Subject: [PATCH 02/13] src: Temporary patch for cx_crc32 (cherry picked from commit 417774925a3b0b7b665c04bd2deee56962f91948) --- src/cx_stubs.S | 4 +++- src/cx_wrappers.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/cx_stubs.S b/src/cx_stubs.S index 9c802f786..2fdba261d 100644 --- a/src/cx_stubs.S +++ b/src/cx_stubs.S @@ -56,7 +56,9 @@ CX_TRAMPOLINE _NR_cx_cmac cx_cmac CX_TRAMPOLINE _NR_cx_constant_time_eq cx_constant_time_eq CX_TRAMPOLINE _NR_cx_crc16 cx_crc16 CX_TRAMPOLINE _NR_cx_crc16_update cx_crc16_update -CX_TRAMPOLINE _NR_cx_crc32 cx_crc32 +// Disable lib CX CRC32 until it get fixed in the OS. +// Meanwhile, it's implemented in cx_wrappers.c +//CX_TRAMPOLINE _NR_cx_crc32 cx_crc32 CX_TRAMPOLINE _NR_cx_decode_coord cx_decode_coord CX_TRAMPOLINE _NR_cx_ecdh_no_throw cx_ecdh_no_throw CX_TRAMPOLINE _NR_cx_ecdsa_sign_no_throw cx_ecdsa_sign_no_throw diff --git a/src/cx_wrappers.c b/src/cx_wrappers.c index f1db2af9a..579846f66 100644 --- a/src/cx_wrappers.c +++ b/src/cx_wrappers.c @@ -60,3 +60,56 @@ cx_err_t cx_ecdsa_sign_rs_no_throw(const cx_ecfp_private_key_t *key, } return error; } + +#define CX_CRC32_INIT 0xFFFFFFFF + +static unsigned int cx_ccitt32[] = { + 0x00000000, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f, 0xe963a535, 0x9e6495a3, + 0x0edb8832, 0x79dcb8a4, 0xe0d5e91e, 0x97d2d988, 0x09b64c2b, 0x7eb17cbd, 0xe7b82d07, 0x90bf1d91, + 0x1db71064, 0x6ab020f2, 0xf3b97148, 0x84be41de, 0x1adad47d, 0x6ddde4eb, 0xf4d4b551, 0x83d385c7, + 0x136c9856, 0x646ba8c0, 0xfd62f97a, 0x8a65c9ec, 0x14015c4f, 0x63066cd9, 0xfa0f3d63, 0x8d080df5, + 0x3b6e20c8, 0x4c69105e, 0xd56041e4, 0xa2677172, 0x3c03e4d1, 0x4b04d447, 0xd20d85fd, 0xa50ab56b, + 0x35b5a8fa, 0x42b2986c, 0xdbbbc9d6, 0xacbcf940, 0x32d86ce3, 0x45df5c75, 0xdcd60dcf, 0xabd13d59, + 0x26d930ac, 0x51de003a, 0xc8d75180, 0xbfd06116, 0x21b4f4b5, 0x56b3c423, 0xcfba9599, 0xb8bda50f, + 0x2802b89e, 0x5f058808, 0xc60cd9b2, 0xb10be924, 0x2f6f7c87, 0x58684c11, 0xc1611dab, 0xb6662d3d, + 0x76dc4190, 0x01db7106, 0x98d220bc, 0xefd5102a, 0x71b18589, 0x06b6b51f, 0x9fbfe4a5, 0xe8b8d433, + 0x7807c9a2, 0x0f00f934, 0x9609a88e, 0xe10e9818, 0x7f6a0dbb, 0x086d3d2d, 0x91646c97, 0xe6635c01, + 0x6b6b51f4, 0x1c6c6162, 0x856530d8, 0xf262004e, 0x6c0695ed, 0x1b01a57b, 0x8208f4c1, 0xf50fc457, + 0x65b0d9c6, 0x12b7e950, 0x8bbeb8ea, 0xfcb9887c, 0x62dd1ddf, 0x15da2d49, 0x8cd37cf3, 0xfbd44c65, + 0x4db26158, 0x3ab551ce, 0xa3bc0074, 0xd4bb30e2, 0x4adfa541, 0x3dd895d7, 0xa4d1c46d, 0xd3d6f4fb, + 0x4369e96a, 0x346ed9fc, 0xad678846, 0xda60b8d0, 0x44042d73, 0x33031de5, 0xaa0a4c5f, 0xdd0d7cc9, + 0x5005713c, 0x270241aa, 0xbe0b1010, 0xc90c2086, 0x5768b525, 0x206f85b3, 0xb966d409, 0xce61e49f, + 0x5edef90e, 0x29d9c998, 0xb0d09822, 0xc7d7a8b4, 0x59b33d17, 0x2eb40d81, 0xb7bd5c3b, 0xc0ba6cad, + 0xedb88320, 0x9abfb3b6, 0x03b6e20c, 0x74b1d29a, 0xead54739, 0x9dd277af, 0x04db2615, 0x73dc1683, + 0xe3630b12, 0x94643b84, 0x0d6d6a3e, 0x7a6a5aa8, 0xe40ecf0b, 0x9309ff9d, 0x0a00ae27, 0x7d079eb1, + 0xf00f9344, 0x8708a3d2, 0x1e01f268, 0x6906c2fe, 0xf762575d, 0x806567cb, 0x196c3671, 0x6e6b06e7, + 0xfed41b76, 0x89d32be0, 0x10da7a5a, 0x67dd4acc, 0xf9b9df6f, 0x8ebeeff9, 0x17b7be43, 0x60b08ed5, + 0xd6d6a3e8, 0xa1d1937e, 0x38d8c2c4, 0x4fdff252, 0xd1bb67f1, 0xa6bc5767, 0x3fb506dd, 0x48b2364b, + 0xd80d2bda, 0xaf0a1b4c, 0x36034af6, 0x41047a60, 0xdf60efc3, 0xa867df55, 0x316e8eef, 0x4669be79, + 0xcb61b38c, 0xbc66831a, 0x256fd2a0, 0x5268e236, 0xcc0c7795, 0xbb0b4703, 0x220216b9, 0x5505262f, + 0xc5ba3bbe, 0xb2bd0b28, 0x2bb45a92, 0x5cb36a04, 0xc2d7ffa7, 0xb5d0cf31, 0x2cd99e8b, 0x5bdeae1d, + 0x9b64c2b0, 0xec63f226, 0x756aa39c, 0x026d930a, 0x9c0906a9, 0xeb0e363f, 0x72076785, 0x05005713, + 0x95bf4a82, 0xe2b87a14, 0x7bb12bae, 0x0cb61b38, 0x92d28e9b, 0xe5d5be0d, 0x7cdcefb7, 0x0bdbdf21, + 0x86d3d2d4, 0xf1d4e242, 0x68ddb3f8, 0x1fda836e, 0x81be16cd, 0xf6b9265b, 0x6fb077e1, 0x18b74777, + 0x88085ae6, 0xff0f6a70, 0x66063bca, 0x11010b5c, 0x8f659eff, 0xf862ae69, 0x616bffd3, 0x166ccf45, + 0xa00ae278, 0xd70dd2ee, 0x4e048354, 0x3903b3c2, 0xa7672661, 0xd06016f7, 0x4969474d, 0x3e6e77db, + 0xaed16a4a, 0xd9d65adc, 0x40df0b66, 0x37d83bf0, 0xa9bcae53, 0xdebb9ec5, 0x47b2cf7f, 0x30b5ffe9, + 0xbdbdf21c, 0xcabac28a, 0x53b39330, 0x24b4a3a6, 0xbad03605, 0xcdd70693, 0x54de5729, 0x23d967bf, + 0xb3667a2e, 0xc4614ab8, 0x5d681b02, 0x2a6f2b94, 0xb40bbe37, 0xc30c8ea1, 0x5a05df1b, 0x2d02ef8d}; + +uint32_t cx_crc32_update(uint32_t crc, const void *buf, size_t len) +{ +#define buffer ((const uint8_t *) buf) + uint32_t i; + + for (i = 0; i < len; i++) { + crc = cx_ccitt32[(crc ^ buffer[i]) & 0xff] ^ (crc >> 8u); + } + + return crc ^ CX_CRC32_INIT; +} + +uint32_t cx_crc32(const void *buf, size_t len) +{ + return cx_crc32_update(CX_CRC32_INIT, buf, len); +} From 005da561a69ef32e054c0f74597b5ef44be11a1a Mon Sep 17 00:00:00 2001 From: Sarah GLINER Date: Tue, 16 Jan 2024 09:46:23 +0100 Subject: [PATCH 03/13] ledger_assert: add header for HAVE_PRINTF (cherry picked from commit f93dffdd9382abbda01518dda0d39266d90ab83f) --- include/ledger_assert.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/ledger_assert.h b/include/ledger_assert.h index 15387e5c4..8141314b4 100644 --- a/include/ledger_assert.h +++ b/include/ledger_assert.h @@ -2,6 +2,10 @@ #include +#ifdef HAVE_PRINTF +#include "os.h" +#endif + #ifdef LEDGER_ASSERT_CONFIG_FILE_INFO #define LEDGER_ASSERT_CONFIG_MESSAGE_INFO 1 #define LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO 1 From f96a8b1a8ca32d265175c13f861b7c5491eed2e6 Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Thu, 4 Jan 2024 09:18:08 +0100 Subject: [PATCH 04/13] Fix compilation warnings (cherry picked from commit 72a78668d723f5af2f0e3db8c468f145a54001c5) --- .../Class/CCID/src/usbd_ccid_cmd.c | 31 +++++++++---------- .../Class/CCID/src/usbd_ccid_if.c | 1 + 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_cmd.c b/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_cmd.c index 31e4759a4..0682de887 100644 --- a/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_cmd.c +++ b/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_cmd.c @@ -231,28 +231,28 @@ uint8_t PC_to_RDR_XfrBlock(void) if (error != 0) return error; - if (G_io_ccid.bulk_header.bulkout.dwLength > IO_CCID_DATA_BUFFER_SIZE) - { /* Check amount of Data Sent by Host is > than memory allocated ? */ + if (G_io_ccid.bulk_header.bulkout.dwLength > IO_CCID_DATA_BUFFER_SIZE) + { /* Check amount of Data Sent by Host is > than memory allocated ? */ - return SLOTERROR_BAD_DWLENGTH; - } + return SLOTERROR_BAD_DWLENGTH; + } - /* wLevelParameter = Size of expected data to be returned by the - bulk-IN endpoint */ - expectedLength = (G_io_ccid.bulk_header.bulkout.bSpecific_2 << 8) | - G_io_ccid.bulk_header.bulkout.bSpecific_1; + /* wLevelParameter = Size of expected data to be returned by the + bulk-IN endpoint */ + expectedLength = (G_io_ccid.bulk_header.bulkout.bSpecific_2 << 8) | + G_io_ccid.bulk_header.bulkout.bSpecific_1; - reqlen = G_io_ccid.bulk_header.bulkout.dwLength; - - G_io_ccid.bulk_header.bulkin.dwLength = (uint16_t)expectedLength; + reqlen = G_io_ccid.bulk_header.bulkout.dwLength; + G_io_ccid.bulk_header.bulkin.dwLength = (uint16_t)expectedLength; - error = SC_XferBlock(&G_io_ccid_data_buffer[0], - reqlen, - &expectedLength); - if (error != SLOT_NO_ERROR) + error = SC_XferBlock(&G_io_ccid_data_buffer[0], + reqlen, + &expectedLength); + + if (error != SLOT_NO_ERROR) { CCID_UpdateCommandStatus(BM_COMMAND_STATUS_FAILED, BM_ICC_PRESENT_ACTIVE); } @@ -639,7 +639,6 @@ uint8_t PC_TO_RDR_SetDataRateAndClockFrequency(void) uint8_t error; uint32_t clockFrequency; uint32_t dataRate; - uint32_t temp =0; error = CCID_CheckCommandParams(CHK_PARAM_SLOT |\ CHK_PARAM_CARD_PRESENT |\ diff --git a/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_if.c b/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_if.c index ef0302615..8b073c2ce 100644 --- a/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_if.c +++ b/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_if.c @@ -206,6 +206,7 @@ void CCID_BulkMessage_Out (USBD_HandleTypeDef *pdev, G_io_ccid.Ccid_BulkState = CCID_STATE_IDLE; // no break is intentional + __attribute__((fallthrough)); case CCID_STATE_IDLE: // prepare to receive another packet later on (to avoid troubles with timeout due to other hid command timeouting the ccid endpoint reply) USBD_LL_PrepareReceive(pdev, CCID_BULK_OUT_EP, CCID_BULK_EPOUT_SIZE); From d9024edc7dd34becf9eb9be33ca2d20c31be8745 Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Thu, 4 Jan 2024 09:22:34 +0100 Subject: [PATCH 05/13] Add pinpad activate function (cherry picked from commit 5b3cda4e9667504fdf029f9cc95bd0897464d387) --- .../Class/CCID/inc/usbd_ccid_if.h | 2 + .../Class/CCID/src/usbd_ccid_if.c | 9 ++- lib_stusb/usbd_conf.h | 1 + lib_stusb_impl/usbd_impl.c | 56 +++++++++++++++++-- 4 files changed, 58 insertions(+), 10 deletions(-) diff --git a/lib_stusb/STM32_USB_Device_Library/Class/CCID/inc/usbd_ccid_if.h b/lib_stusb/STM32_USB_Device_Library/Class/CCID/inc/usbd_ccid_if.h index 9326da8ff..7cd8ecd39 100644 --- a/lib_stusb/STM32_USB_Device_Library/Class/CCID/inc/usbd_ccid_if.h +++ b/lib_stusb/STM32_USB_Device_Library/Class/CCID/inc/usbd_ccid_if.h @@ -223,6 +223,8 @@ void Set_CSW (uint8_t CSW_Status, uint8_t Send_Permission); void io_usb_ccid_set_card_inserted(unsigned int inserted); +void io_usb_ccid_configure_pinpad(uint8_t enabled); + #endif // HAVE_USB_CLASS_CCID #endif /* __USBD_CCID_IF_H */ diff --git a/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_if.c b/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_if.c index 8b073c2ce..eaa83b816 100644 --- a/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_if.c +++ b/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_if.c @@ -615,11 +615,10 @@ void io_usb_ccid_set_card_inserted(unsigned int inserted) { #endif // HAVE_CCID_INTERRUPT } - - - - - +void io_usb_ccid_configure_pinpad(uint8_t enabled) { + const volatile uint8_t *cfgDesc = USBD_GetPinPadOffset(); + nvm_write((void *)cfgDesc, &enabled, 1); +} #endif // HAVE_USB_CLASS_CCID diff --git a/lib_stusb/usbd_conf.h b/lib_stusb/usbd_conf.h index c58b7f73e..886496c86 100644 --- a/lib_stusb/usbd_conf.h +++ b/lib_stusb/usbd_conf.h @@ -167,6 +167,7 @@ typedef unsigned short uint16_t; void *USBD_static_malloc(uint32_t size); void USBD_static_free(void *p); +const volatile uint8_t *USBD_GetPinPadOffset(void); void USB_power(unsigned char enabled); diff --git a/lib_stusb_impl/usbd_impl.c b/lib_stusb_impl/usbd_impl.c index 7fdfac406..70b6c6d65 100644 --- a/lib_stusb_impl/usbd_impl.c +++ b/lib_stusb_impl/usbd_impl.c @@ -353,21 +353,34 @@ static uint8_t const HID_ReportDesc_fido[] = { #define ARRAY_U2LE(l) (l)&0xFF, (l)>>8 +#define CFG_HDR_LEN (0x9) +#define CFG_HIDGEN_LEN (0x9+0x9+0x7+0x7) +#define CFG_IO_U2F_LEN (0x9+0x9+0x7+0x7) +#define CFG_USB_CCID_LEN (0x9+0x36+0x7+0x7) +#define CFG_WEBUSB_LEN (0x9+0x7+0x7) + /* USB HID device Configuration Descriptor */ +#ifdef HAVE_USB_CLASS_CCID +// Note: keeping const qualifier to ensure the good section is used by the linker. +// This table is mapped in NVRAM section and therefore it is correctly initialized, +// normal Read is possible and Write can be done through nvm_write() calls. +static __ALIGN_BEGIN uint8_t const N_USBD_CfgDesc[] __ALIGN_END = +#else static __ALIGN_BEGIN uint8_t const USBD_CfgDesc[] __ALIGN_END = +#endif // HAVE_USB_CLASS_CCID { 0x09, /* bLength: Configuration Descriptor size */ USB_DESC_TYPE_CONFIGURATION, /* bDescriptorType: Configuration */ - ARRAY_U2LE(0x9 /* wTotalLength: Bytes returned */ - +0x9+0x9+0x7+0x7 + ARRAY_U2LE(CFG_HDR_LEN /* wTotalLength: Bytes returned */ + +CFG_HIDGEN_LEN #ifdef HAVE_IO_U2F - +0x9+0x9+0x7+0x7 + +CFG_IO_U2F_LEN #endif // HAVE_IO_U2F #ifdef HAVE_USB_CLASS_CCID - +0x9+0x36+0x7+0x7 + +CFG_USB_CCID_LEN #endif // HAVE_USB_CLASS_CCID #ifdef HAVE_WEBUSB - +0x9+0x7+0x7 + +CFG_WEBUSB_LEN #endif // HAVE_WEBUSB ), 1 @@ -870,8 +883,13 @@ static uint8_t *USBD_GetDeviceQualifierDesc_impl(uint16_t *length) */ static uint8_t *USBD_GetCfgDesc_impl(uint16_t *length) { +#ifdef HAVE_USB_CLASS_CCID + *length = sizeof(N_USBD_CfgDesc); + return (uint8_t *) N_USBD_CfgDesc; +#else *length = sizeof(USBD_CfgDesc); return (uint8_t *) USBD_CfgDesc; +#endif } uint8_t *USBD_HID_GetHidDescriptor_impl(uint16_t *len) @@ -927,6 +945,34 @@ uint8_t *USBD_HID_GetReportDescriptor_impl(uint16_t *len) return 0; } +#ifdef HAVE_USB_CLASS_CCID +/** + * @brief Returns the pinpad value offset in the descriptor. + * @retval Offset + */ +const volatile uint8_t *USBD_GetPinPadOffset(void) +{ + unsigned short length = 0; + uint8_t *cfgDesc = NULL; + unsigned short offset = 0; + + cfgDesc = USBD_GetCfgDesc_impl(&length); + + offset = CFG_HDR_LEN + CFG_HIDGEN_LEN; +#ifdef HAVE_IO_U2F + offset += CFG_IO_U2F_LEN; +#endif // HAVE_IO_U2F + // Offset of the parameter 'bPINSupport' inside the CCID interface structure in N_USBD_CfgDesc + offset += 61; + + // Returns a const volatile pointer allowing callers to do + // - write operations through nvram_write + // - read operation without potential compilation optimization issue thanks to the volatile + // qualifier. + return (const volatile uint8_t *) (cfgDesc + offset); +} +#endif // HAVE_USB_CLASS_CCID + /** * @} */ From 0114a960c199a6387f268361d35e81cef79a57c1 Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Thu, 4 Jan 2024 09:25:19 +0100 Subject: [PATCH 06/13] Update SC_Secure to fit with CCID spec; remove useless parameters (cherry picked from commit 1bdd75945ab24af7b40d8d90bedd02931e3879cd) --- .../Class/CCID/inc/sc_itf.h | 5 +- .../Class/CCID/inc/usbd_ccid_if.h | 8 +++ .../Class/CCID/src/usbd_ccid_cmd.c | 12 +--- .../Class/CCID/src/usbd_ccid_if.c | 56 ++++++++++--------- 4 files changed, 44 insertions(+), 37 deletions(-) diff --git a/lib_stusb/STM32_USB_Device_Library/Class/CCID/inc/sc_itf.h b/lib_stusb/STM32_USB_Device_Library/Class/CCID/inc/sc_itf.h index dfac8d607..8babdec31 100644 --- a/lib_stusb/STM32_USB_Device_Library/Class/CCID/inc/sc_itf.h +++ b/lib_stusb/STM32_USB_Device_Library/Class/CCID/inc/sc_itf.h @@ -64,7 +64,7 @@ uint8_t SC_ExecuteEscape (uint8_t* escapePtr, uint32_t escapeLen, uint8_t* responseBuff, uint16_t* responseLen); uint8_t SC_SetClock (uint8_t bClockCommand); -uint8_t SC_XferBlock (uint8_t* ptrBlock, uint32_t blockLen, uint16_t* expectedLen); +uint8_t SC_XferBlock (uint8_t* ptrBlock, uint32_t blockLen); uint8_t SC_Request_GetClockFrequencies(uint8_t* pbuf, uint16_t* len); uint8_t SC_Request_GetDataRates(uint8_t* pbuf, uint16_t* len); uint8_t SC_T0Apdu(uint8_t bmChanges, uint8_t bClassGetResponse, @@ -72,8 +72,7 @@ uint8_t SC_T0Apdu(uint8_t bmChanges, uint8_t bClassGetResponse, uint8_t SC_Mechanical(uint8_t bFunction); uint8_t SC_SetDataRateAndClockFrequency(uint32_t dwClockFrequency, uint32_t dwDataRate); -uint8_t SC_Secure(uint32_t dwLength, uint8_t bBWI, uint16_t wLevelParameter, - uint8_t* pbuf, uint32_t* returnLen ); +uint8_t SC_Secure(uint8_t* pbuf, uint32_t* returnLen); #endif // HAVE_USB_CLASS_CCID diff --git a/lib_stusb/STM32_USB_Device_Library/Class/CCID/inc/usbd_ccid_if.h b/lib_stusb/STM32_USB_Device_Library/Class/CCID/inc/usbd_ccid_if.h index 7cd8ecd39..1e077a7c8 100644 --- a/lib_stusb/STM32_USB_Device_Library/Class/CCID/inc/usbd_ccid_if.h +++ b/lib_stusb/STM32_USB_Device_Library/Class/CCID/inc/usbd_ccid_if.h @@ -58,6 +58,14 @@ #define VOLTAGE_SELECTION_5V 0x01 #define VOLTAGE_SELECTION_1V8 0x03 +/* PC_to_RDR_Secure PIN operations */ +#define PIN_OPR_VERIFICATION 0x00 +#define PIN_OPR_MODIFICATION 0x01 +#define PIN_OPR_TRANSFER 0x02 +#define PIN_OPR_WAIT_ICC_RESP 0x03 +#define PIN_OPR_CANCEL 0x04 +#define PIN_OPR_APDU_CLA 0xEF + #define PC_TO_RDR_ICCPOWERON 0x62 #define PC_TO_RDR_ICCPOWEROFF 0x63 #define PC_TO_RDR_GETSLOTSTATUS 0x65 diff --git a/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_cmd.c b/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_cmd.c index 0682de887..e75501c60 100644 --- a/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_cmd.c +++ b/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_cmd.c @@ -247,10 +247,7 @@ uint8_t PC_to_RDR_XfrBlock(void) G_io_ccid.bulk_header.bulkin.dwLength = (uint16_t)expectedLength; - - error = SC_XferBlock(&G_io_ccid_data_buffer[0], - reqlen, - &expectedLength); + error = SC_XferBlock(&G_io_ccid_data_buffer[0], reqlen); if (error != SLOT_NO_ERROR) { @@ -693,7 +690,6 @@ uint8_t PC_TO_RDR_SetDataRateAndClockFrequency(void) uint8_t PC_TO_RDR_Secure(void) { uint8_t error; - uint8_t bBWI; uint16_t wLevelParameter; uint32_t responseLen; @@ -707,14 +703,13 @@ uint8_t PC_TO_RDR_Secure(void) return error; } - bBWI = G_io_ccid.bulk_header.bulkout.bSpecific_0; wLevelParameter = (G_io_ccid.bulk_header.bulkout.bSpecific_1 + ((uint16_t)G_io_ccid.bulk_header.bulkout.bSpecific_2<<8)); if ((EXCHANGE_LEVEL_FEATURE == TPDU_EXCHANGE) || (EXCHANGE_LEVEL_FEATURE == SHORT_APDU_EXCHANGE)) { /* TPDU level & short APDU level, wLevelParameter is RFU, = 0000h */ - if (wLevelParameter != 0 ) + if (wLevelParameter != 0) { G_io_ccid.bulk_header.bulkin.dwLength = 0; CCID_UpdateCommandStatus(BM_COMMAND_STATUS_FAILED, BM_ICC_PRESENT_ACTIVE); @@ -723,8 +718,7 @@ uint8_t PC_TO_RDR_Secure(void) } } - error = SC_Secure(G_io_ccid.bulk_header.bulkout.dwLength - CCID_HEADER_SIZE, bBWI, wLevelParameter, - &G_io_ccid_data_buffer[0], &responseLen); + error = SC_Secure(&G_io_ccid_data_buffer[0], &responseLen); G_io_ccid.bulk_header.bulkin.dwLength = responseLen; diff --git a/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_if.c b/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_if.c index eaa83b816..fc05b9466 100644 --- a/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_if.c +++ b/lib_stusb/STM32_USB_Device_Library/Class/CCID/src/usbd_ccid_if.c @@ -526,35 +526,39 @@ uint8_t SC_SetDataRateAndClockFrequency(uint32_t dwClockFrequency, UNUSED(dwDataRate); return SLOT_NO_ERROR; } -uint8_t SC_Secure(uint32_t dwLength, uint8_t bBWI, uint16_t wLevelParameter, - uint8_t* pbuf, uint32_t* returnLen ) { - UNUSED(bBWI); - UNUSED(wLevelParameter); - UNUSED(returnLen); - // return SLOTERROR_CMD_NOT_SUPPORTED; - uint16_t ret_len,off; +uint8_t SC_Secure(uint8_t* pbuf, uint32_t* returnLen) { + // Extract the APDU to send to the App switch(pbuf[0]) { - case 0: // verify pin - ret_len = dwLength - 15; - memmove(G_io_apdu_buffer, pbuf+15, dwLength-15); + case PIN_OPR_VERIFICATION: + // CCID Spec: APDU starts at offset 25, after the 10-Byte header + pbuf += 15; break; - case 1: // modify pin + case PIN_OPR_MODIFICATION: + // CCID Spec: APDU starts at offset 28, 29 or 30 + // depending on the nb of messages to display switch(pbuf[11]) { - case 3: - off = 20; + case 0: + // CCID Spec: No message to display + // APDU starts at offset 28, after the 10-Byte header + pbuf += 18; break; - case 2: case 1: - off = 19; + case 2: + // CCID Spec: 1 or 2 message(s) to display + // APDU starts at offset 29, after the 10-Byte header + pbuf += 19; break; - // 0 and 4-0xFF - default: - off = 18; + case 3: + // CCID Spec: 3 messages to display + // APDU starts at offset 30, after the 10-Byte header + pbuf += 20; break; + default: // unsupported + G_io_ccid.bulk_header.bulkin.dwLength = 0; + RDR_to_PC_DataBlock(SLOTERROR_CMD_NOT_SUPPORTED); + CCID_Send_Reply(&USBD_Device); + return SLOTERROR_CMD_NOT_SUPPORTED; } - ret_len = dwLength-off; - // provide with the complete apdu - memmove(G_io_apdu_buffer, pbuf+off, dwLength-off); break; default: // unsupported G_io_ccid.bulk_header.bulkin.dwLength = 0; @@ -562,13 +566,15 @@ uint8_t SC_Secure(uint32_t dwLength, uint8_t bBWI, uint16_t wLevelParameter, CCID_Send_Reply(&USBD_Device); return SLOTERROR_CMD_NOT_SUPPORTED; } - return SC_XferBlock(G_io_apdu_buffer, ret_len, &ret_len); + // Change APDU CLA to be interpreted by the CCID compatible App (like OpenPGP) + pbuf[0] = PIN_OPR_APDU_CLA; + // The APDU has no data, only the header (size 5) + *returnLen = 5; + return SC_XferBlock(pbuf, *returnLen); } // prepare the apdu to be processed by the application -uint8_t SC_XferBlock (uint8_t* ptrBlock, uint32_t blockLen, uint16_t* expectedLen) { - UNUSED(expectedLen); - +uint8_t SC_XferBlock (uint8_t* ptrBlock, uint32_t blockLen) { // check for overflow if (blockLen > IO_APDU_BUFFER_SIZE) { return SLOTERROR_BAD_LENTGH; From 4852e98dc40745b59d5134e27c5e37943ecddd5f Mon Sep 17 00:00:00 2001 From: Xavier Chapron Date: Thu, 18 Jan 2024 15:24:04 +0100 Subject: [PATCH 07/13] ledger_assert: Fix circular dependency (cherry picked from commit a501ab9ba1edb2194dbab01b62d2edd7a798d666) --- include/ledger_assert.h | 4 ++-- src/ledger_assert.c | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/include/ledger_assert.h b/include/ledger_assert.h index 8141314b4..776c30c69 100644 --- a/include/ledger_assert.h +++ b/include/ledger_assert.h @@ -3,7 +3,7 @@ #include #ifdef HAVE_PRINTF -#include "os.h" +void assert_print_failed(void); #endif #ifdef LEDGER_ASSERT_CONFIG_FILE_INFO @@ -97,7 +97,7 @@ void assert_print_file_info(const char *file, int line); ASSERT_DISPLAY_LR_AND_PC(_lr_address, _pc_address); \ } while (0) #elif defined(HAVE_PRINTF) -#define LEDGER_ASSERT_LR_AND_PC() PRINTF("LEDGER_ASSERT FAILED\n") +#define LEDGER_ASSERT_LR_AND_PC() assert_print_failed() #else #define LEDGER_ASSERT_LR_AND_PC() \ do { \ diff --git a/src/ledger_assert.c b/src/ledger_assert.c index d39b76811..fa7022e62 100644 --- a/src/ledger_assert.c +++ b/src/ledger_assert.c @@ -35,6 +35,13 @@ static char assert_buffer[ASSERT_BUFFER_LEN]; #endif +#if defined(HAVE_PRINTF) +void assert_print_failed(void) +{ + PRINTF("LEDGER_ASSERT FAILED\n"); +} +#endif + #if defined(HAVE_LEDGER_ASSERT_DISPLAY) && defined(LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO) void assert_display_lr_and_pc(int lr, int pc) { From ffb8fdf53e9569c94dd90e615d52f6feb59ca06a Mon Sep 17 00:00:00 2001 From: Xavier Chapron Date: Mon, 22 Jan 2024 11:47:44 +0100 Subject: [PATCH 08/13] os_print.h: Move PRINTF declaration in dedicated header (cherry picked from commit d345ef6829209e963ff57f2eece5b17a78d19c4e) --- include/os.h | 7 ------- include/os_print.h | 7 +++++++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/os.h b/include/os.h index 27633c383..a329e07b8 100644 --- a/include/os.h +++ b/include/os.h @@ -117,13 +117,6 @@ os is not an application (it calls ux upon user inputs) /* ----------------------------------------------------------------------- */ /* - DEBUG FUNCTIONS - */ /* ----------------------------------------------------------------------- */ -#ifdef HAVE_PRINTF -void screen_printf(const char *format, ...); -void mcu_usb_printf(const char *format, ...); -#else // !HAVE_PRINTF -#define PRINTF(...) -#endif // !HAVE_PRINTF - // redefined if string.h not included #ifdef HAVE_SPRINTF #ifndef __APPLE__ diff --git a/include/os_print.h b/include/os_print.h index 1480becf2..9e0d293cc 100644 --- a/include/os_print.h +++ b/include/os_print.h @@ -4,3 +4,10 @@ // avoid typing the size each time #define SPRINTF(strbuf, ...) snprintf(strbuf, sizeof(strbuf), __VA_ARGS__) + +#ifdef HAVE_PRINTF +void screen_printf(const char *format, ...); +void mcu_usb_printf(const char *format, ...); +#else // !HAVE_PRINTF +#define PRINTF(...) +#endif // !HAVE_PRINTF From 4991c4040a1cc1a5c500784615a81c48ef5054e4 Mon Sep 17 00:00:00 2001 From: Xavier Chapron Date: Tue, 23 Jan 2024 14:35:03 +0100 Subject: [PATCH 09/13] ledger_assert: Add support for printf like message and add documentation (cherry picked from commit 9ba66cebeace8df284ada336a83c239c64f97c1f) --- include/ledger_assert.h | 249 +++++++++++------------------- include/ledger_assert_internals.h | 226 +++++++++++++++++++++++++++ src/ledger_assert.c | 74 ++++----- 3 files changed, 351 insertions(+), 198 deletions(-) create mode 100644 include/ledger_assert_internals.h diff --git a/include/ledger_assert.h b/include/ledger_assert.h index 776c30c69..c3b714a64 100644 --- a/include/ledger_assert.h +++ b/include/ledger_assert.h @@ -1,160 +1,95 @@ #pragma once -#include - -#ifdef HAVE_PRINTF -void assert_print_failed(void); -#endif - -#ifdef LEDGER_ASSERT_CONFIG_FILE_INFO -#define LEDGER_ASSERT_CONFIG_MESSAGE_INFO 1 -#define LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO 1 -#endif - -#ifdef LEDGER_ASSERT_CONFIG_MESSAGE_INFO -#define LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO 1 -#endif - -#if defined(LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO) && defined(HAVE_LEDGER_ASSERT_DISPLAY) -#define LR_AND_PC_SIZE 30 -void assert_display_lr_and_pc(int lr, int pc); -#define ASSERT_DISPLAY_LR_AND_PC(lr, pc) assert_display_lr_and_pc(lr, pc) -#else -#define LR_AND_PC_SIZE 0 -#define ASSERT_DISPLAY_LR_AND_PC(lr, pc) \ - do { \ - } while (0) -#endif - -#if defined(LEDGER_ASSERT_CONFIG_MESSAGE_INFO) && defined(HAVE_LEDGER_ASSERT_DISPLAY) -#define MESSAGE_SIZE 20 -void assert_display_message(const char *message); -#define ASSERT_DISPLAY_MESSAGE(message) assert_display_message(message) -#else -#define MESSAGE_SIZE 0 -#define ASSERT_DISPLAY_MESSAGE(message) \ - do { \ - } while (0) -#endif - -#if defined(LEDGER_ASSERT_CONFIG_FILE_INFO) && defined(HAVE_LEDGER_ASSERT_DISPLAY) -#define FILE_SIZE 50 -void assert_display_file_info(const char *file, unsigned int line); -#define ASSERT_DISPLAY_FILE_INFO(file, line) assert_display_file_info(file, line) -#else -#define FILE_SIZE 0 -#define ASSERT_DISPLAY_FILE_INFO(file, line) \ - do { \ - } while (0) -#endif - -#ifdef HAVE_LEDGER_ASSERT_DISPLAY -#define ASSERT_BUFFER_LEN LR_AND_PC_SIZE + MESSAGE_SIZE + FILE_SIZE -void __attribute__((noreturn)) assert_display_exit(void); - -#define LEDGER_ASSERT_EXIT() assert_display_exit() -#else -void assert_exit(bool confirm); -#define LEDGER_ASSERT_EXIT() assert_exit(true) -#endif - -#if defined(LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO) && defined(HAVE_PRINTF) -void assert_print_lr_and_pc(int lr, int pc); -#define ASSERT_PRINT_LR_AND_PC(lr, pc) assert_print_lr_and_pc(lr, pc) -#else -#define ASSERT_PRINT_LR_AND_PC(lr, pc) \ - do { \ - } while (0) -#endif - -#if defined(LEDGER_ASSERT_CONFIG_MESSAGE_INFO) && defined(HAVE_PRINTF) -void assert_print_message(const char *message); -#define ASSERT_PRINT_MESSAGE(message) assert_print_message(message) -#else -#define ASSERT_PRINT_MESSAGE(message) \ - do { \ - } while (0) -#endif - -#if defined(LEDGER_ASSERT_CONFIG_FILE_INFO) && defined(HAVE_PRINTF) -void assert_print_file_info(const char *file, int line); -#define ASSERT_PRINT_FILE_INFO(file, line) assert_print_file_info(file, line) -#else -#define ASSERT_PRINT_FILE_INFO(file, line) \ - do { \ - } while (0) -#endif - -#ifdef LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO -#define LEDGER_ASSERT_LR_AND_PC() \ - do { \ - int _lr_address = 0; \ - int _pc_address = 0; \ - \ - __asm volatile("mov %0, lr" : "=r"(_lr_address)); \ - __asm volatile("mov %0, pc" : "=r"(_pc_address)); \ - ASSERT_PRINT_LR_AND_PC(_lr_address, _pc_address); \ - ASSERT_DISPLAY_LR_AND_PC(_lr_address, _pc_address); \ - } while (0) -#elif defined(HAVE_PRINTF) -#define LEDGER_ASSERT_LR_AND_PC() assert_print_failed() -#else -#define LEDGER_ASSERT_LR_AND_PC() \ - do { \ - } while (0) -#endif - -#ifdef LEDGER_ASSERT_CONFIG_MESSAGE_INFO -#define LEDGER_ASSERT_MESSAGE(message) \ - do { \ - ASSERT_PRINT_MESSAGE(message); \ - ASSERT_DISPLAY_MESSAGE(message); \ - } while (0) -#else -#define LEDGER_ASSERT_MESSAGE(message) \ - do { \ - } while (0) -#endif - -#ifdef LEDGER_ASSERT_CONFIG_FILE_INFO -#define LEDGER_ASSERT_FILE_INFO() \ - do { \ - ASSERT_PRINT_FILE_INFO(__FILE__, __LINE__); \ - ASSERT_DISPLAY_FILE_INFO(__FILE__, __LINE__); \ - } while (0) -#else -#define LEDGER_ASSERT_FILE_INFO() \ - do { \ - } while (0) -#endif - -#define LEDGER_ASSERT(test, message) \ - do { \ - if (!(test)) { \ - LEDGER_ASSERT_LR_AND_PC(); \ - LEDGER_ASSERT_MESSAGE(message); \ - LEDGER_ASSERT_FILE_INFO(); \ - LEDGER_ASSERT_EXIT(); \ - } \ - } while (0) - -#if defined(HAVE_DEBUG_THROWS) && defined(HAVE_PRINTF) -void throw_print_lr(int e, int lr); -#define THROW_PRINT_LR(e, lr_val) throw_print_lr(e, lr_val) -#else -#define THROW_PRINT_LR(e, lr_val) \ - do { \ - } while (0) -#endif - -#if defined(HAVE_DEBUG_THROWS) -void __attribute__((noreturn)) assert_display_exit(void); -void throw_display_lr(int e, int lr); -#define DEBUG_THROW(e) \ - do { \ - unsigned int lr_val; \ - __asm volatile("mov %0, lr" : "=r"(lr_val)); \ - throw_display_lr(e, lr_val); \ - THROW_PRINT_LR(e, lr_val); \ +#include "ledger_assert_internals.h" + +/***************** + * LEDGER_ASSERT * + ****************/ + +/** + * LEDGER_ASSERT - exit the app if assertion is false. + * + * Description: + * This is a C macro that can be used similarly of the libc `assert` macro. + * Therefore, it can help programmers find bugs in their programs, or handle + * exceptional cases via a crash that will produce limited debugging output. + * + * + * Important note: + * Note that contrary to the libc `assert`, the `LEDGER_ASSERT` macro will + * still end the app execution even if build with DEBUG=0 or NDEBUG. + * However, there are configurations explained below that shall be used to + * reduce the code size impact of the `LEDGER_ASSERT` macro to the bare minimum. + * + * + * Examples of usage: + * + * 1/ Simple example always raising with simple message: + * - `LEDGER_ASSERT(false, "Always assert");` + * + * 2/ More complex examples: + * - `LEDGER_ASSERT(len <= sizeof(buf), "Len too long");` + * - `LEDGER_ASSERT(check_value(value) == 0, "check_value failed");` + * + * 3/ Examples showcasing advance message format: + * - `LEDGER_ASSERT(len <= sizeof(buf), "Len too long %d <= %d", len, sizeof(buf));` + * - `int err = check_value(value); LEDGER_ASSERT(err == 0, "check_value failed (%d)", err);` + * + * + * Configuration: + * + * I/ Type of info to expose in PRINTF and screen if enabled. + * + * There are 4 types of information that can be displayed: + * + * 1/ A simple info displaying "LEDGER_ASSERT FAILED" + * => This is always enabled + * + * 2/ An info containing the PC and LR of the instruction that failed. + * This can be used to locate the failing code and flow using the following + * command: `arm-none-eabi-addr2line --exe bin/app.elf -pf 0xC0DE586B` + * => This is enabled when `LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO` is defined. + * + * 3/ An info displaying the `LEDGER_ASSERT` message passed as parameter. + * => This is enabled when `LEDGER_ASSERT_CONFIG_MESSAGE_INFO` is defined. + * + * 4/ An info displaying the file and line position of the failing + * `LEDGER_ASSERT`. + * => This is enabled when `LEDGER_ASSERT_CONFIG_FILE_INFO` is defined. + * + * By default, when an info level X is enabled, all info level below are enabled. + * + * When using `Makefile.standard_app` and building with `DEBUG=1` all info are + * enabled. + * + * Note that enabling these info increase the app code size and is only + * recommended to ease the debugging. + * + * + * II/ Display info on device screen. + * + * Control whether or not a specific screen displaying assert info is shown + * to the app user before exiting the app due to an assert failure. + * + * To enable it, add `DEFINES+=HAVE_LEDGER_ASSERT_DISPLAY` in your app Makefile. + * + * This is enabled by default when using `Makefile.standard_app` and building + * with `DEBUG=1`. It can still be disabled with using + * `DISABLE_DEBUG_LEDGER_ASSERT=1`. + * + * Note that this is increasing the app code size and exposes a non-user + * friendly screen in case of assert. therefore this should not be enabled in + * app release build but only kept for debug purposes. + * + */ +#define LEDGER_ASSERT(test, format, ...) \ + do { \ + if (!(test)) { \ + LEDGER_ASSERT_LR_AND_PC_PREAMBLE(); \ + PRINTF("LEDGER_ASSERT FAILED\n"); \ + LEDGER_ASSERT_MESSAGE(format, ##__VA_ARGS__); \ + LEDGER_ASSERT_FILE_INFO(); \ + LEDGER_ASSERT_LR_AND_PC(); \ + LEDGER_ASSERT_EXIT(); \ + } \ } while (0) -#endif diff --git a/include/ledger_assert_internals.h b/include/ledger_assert_internals.h new file mode 100644 index 000000000..ddf8e2eb1 --- /dev/null +++ b/include/ledger_assert_internals.h @@ -0,0 +1,226 @@ +#pragma once + +#include +#include +#include +#include "os_print.h" + +/******************************* + * Default configuration level * + ******************************/ +#ifdef LEDGER_ASSERT_CONFIG_FILE_INFO +#define LEDGER_ASSERT_CONFIG_MESSAGE_INFO 1 +#define LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO 1 +#endif + +#ifdef LEDGER_ASSERT_CONFIG_MESSAGE_INFO +#define LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO 1 +#endif + +/********************************************************************* + * Compute needed buffer length to display the assert info on screen * + ********************************************************************/ +#ifdef HAVE_LEDGER_ASSERT_DISPLAY + +#ifdef LEDGER_ASSERT_CONFIG_MESSAGE_INFO +#if defined(TARGET_NANOS) +// Spare RAM on NANOS +#define MESSAGE_SIZE 20 +#else +#define MESSAGE_SIZE 50 +#endif +#else +#define MESSAGE_SIZE 0 +#endif + +#ifdef LEDGER_ASSERT_CONFIG_FILE_INFO +#define FILE_SIZE 50 +#else +#define FILE_SIZE 0 +#endif + +#ifdef LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO +#define LR_AND_PC_SIZE 30 +#else +#define LR_AND_PC_SIZE 0 +#endif + +#define ASSERT_BUFFER_LEN LR_AND_PC_SIZE + MESSAGE_SIZE + FILE_SIZE +extern char assert_buffer[ASSERT_BUFFER_LEN]; + +#endif // HAVE_LEDGER_ASSERT_DISPLAY + +/************************************************************ + * Define behavior when LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO * + ***********************************************************/ +#ifdef LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO + +#ifdef HAVE_PRINTF +void assert_print_lr_and_pc(int lr, int pc); +#define ASSERT_PRINT_LR_AND_PC(lr, pc) assert_print_lr_and_pc(lr, pc) +#else +#define ASSERT_PRINT_LR_AND_PC(lr, pc) \ + do { \ + } while (0) +#endif + +#ifdef HAVE_LEDGER_ASSERT_DISPLAY +void assert_display_lr_and_pc(int lr, int pc); +#define ASSERT_DISPLAY_LR_AND_PC(lr, pc) assert_display_lr_and_pc(lr, pc) +#else +#define ASSERT_DISPLAY_LR_AND_PC(lr, pc) \ + do { \ + } while (0) +#endif + +#define LEDGER_ASSERT_LR_AND_PC_PREAMBLE() \ + int _lr_address = 0; \ + int _pc_address = 0; \ + \ + __asm volatile("mov %0, lr" : "=r"(_lr_address)); \ + __asm volatile("mov %0, pc" : "=r"(_pc_address)) + +#define LEDGER_ASSERT_LR_AND_PC() \ + do { \ + ASSERT_PRINT_LR_AND_PC(_lr_address, _pc_address); \ + ASSERT_DISPLAY_LR_AND_PC(_lr_address, _pc_address); \ + } while (0) + +#else // LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO + +#define LEDGER_ASSERT_LR_AND_PC_PREAMBLE() \ + do { \ + } while (0) + +#define LEDGER_ASSERT_LR_AND_PC() \ + do { \ + } while (0) + +#endif // LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO + +/********************************************************** + * Define behavior when LEDGER_ASSERT_CONFIG_MESSAGE_INFO * + *********************************************************/ +#ifdef LEDGER_ASSERT_CONFIG_MESSAGE_INFO + +#ifdef HAVE_PRINTF +#define ASSERT_PRINT_MESSAGE(format, ...) \ + do { \ + PRINTF(format, ##__VA_ARGS__); \ + PRINTF("\n"); \ + } while (0) +#else +#define ASSERT_PRINT_MESSAGE(...) \ + do { \ + } while (0) +#endif + +#ifdef HAVE_LEDGER_ASSERT_DISPLAY +// Immediately call snprintf here (no function wrapping it cleanly in ledger_assert.c). +// This is because we don't have an vsnprintf implementation which would be needed if +// we were to pass the va_args to an intermediate function. +// See https://stackoverflow.com/a/150578 +#define ASSERT_DISPLAY_MESSAGE(format, ...) \ + do { \ + snprintf(assert_buffer, MESSAGE_SIZE, format, ##__VA_ARGS__); \ + strncat(assert_buffer, "\n", MESSAGE_SIZE); \ + } while (0) +#else +#define ASSERT_DISPLAY_MESSAGE(format, ...) \ + do { \ + } while (0) +#endif + +#define LEDGER_ASSERT_MESSAGE(format, ...) \ + do { \ + if (format != NULL) { \ + ASSERT_PRINT_MESSAGE(format, ##__VA_ARGS__); \ + ASSERT_DISPLAY_MESSAGE(format, ##__VA_ARGS__); \ + } \ + } while (0) + +#else // LEDGER_ASSERT_CONFIG_MESSAGE_INFO + +#define LEDGER_ASSERT_MESSAGE(...) \ + do { \ + } while (0) + +#endif // LEDGER_ASSERT_CONFIG_MESSAGE_INFO + +/******************************************************* + * Define behavior when LEDGER_ASSERT_CONFIG_FILE_INFO * + ******************************************************/ +#ifdef LEDGER_ASSERT_CONFIG_FILE_INFO + +#ifdef HAVE_PRINTF +void assert_print_file_info(const char *file, int line); +#define ASSERT_PRINT_FILE_INFO(file, line) assert_print_file_info(file, line) +#else +#define ASSERT_PRINT_FILE_INFO(file, line) \ + do { \ + } while (0) +#endif + +#ifdef HAVE_LEDGER_ASSERT_DISPLAY +void assert_display_file_info(const char *file, unsigned int line); +#define ASSERT_DISPLAY_FILE_INFO(file, line) assert_display_file_info(file, line) +#else +#define ASSERT_DISPLAY_FILE_INFO(file, line) \ + do { \ + } while (0) +#endif + +#define LEDGER_ASSERT_FILE_INFO() \ + do { \ + ASSERT_PRINT_FILE_INFO(__FILE__, __LINE__); \ + ASSERT_DISPLAY_FILE_INFO(__FILE__, __LINE__); \ + } while (0) + +#else // LEDGER_ASSERT_CONFIG_FILE_INFO + +#define LEDGER_ASSERT_FILE_INFO() \ + do { \ + } while (0) + +#endif // LEDGER_ASSERT_CONFIG_FILE_INFO + +/******************************************************** + * Define exit behavior when HAVE_LEDGER_ASSERT_DISPLAY * + *******************************************************/ +#ifdef HAVE_LEDGER_ASSERT_DISPLAY +void __attribute__((noreturn)) assert_display_exit(void); +#define LEDGER_ASSERT_EXIT() assert_display_exit() + +#else // HAVE_LEDGER_ASSERT_DISPLAY + +void assert_exit(bool confirm); +#define LEDGER_ASSERT_EXIT() assert_exit(true) + +#endif // HAVE_LEDGER_ASSERT_DISPLAY + +/************************************* + * Specific mechanism to debug THROW * + ************************************/ +#ifdef HAVE_DEBUG_THROWS + +#ifdef HAVE_PRINTF +void throw_print_lr(int e, int lr); +#define THROW_PRINT_LR(e, lr_val) throw_print_lr(e, lr_val) +#else +#define THROW_PRINT_LR(e, lr_val) \ + do { \ + } while (0) +#endif + +void __attribute__((noreturn)) assert_display_exit(void); +void throw_display_lr(int e, int lr); + +#define DEBUG_THROW(e) \ + do { \ + unsigned int lr_val; \ + __asm volatile("mov %0, lr" : "=r"(lr_val)); \ + throw_display_lr(e, lr_val); \ + THROW_PRINT_LR(e, lr_val); \ + } while (0) + +#endif // HAVE_DEBUG_THROWS diff --git a/src/ledger_assert.c b/src/ledger_assert.c index fa7022e62..3ba6619c9 100644 --- a/src/ledger_assert.c +++ b/src/ledger_assert.c @@ -27,22 +27,19 @@ #include "nbgl_use_case.h" #endif +#if defined(HAVE_LEDGER_ASSERT_DISPLAY) || defined(HAVE_DEBUG_THROWS) #ifndef ASSERT_BUFFER_LEN #define ASSERT_BUFFER_LEN 24 #endif -#if defined(HAVE_LEDGER_ASSERT_DISPLAY) || defined(HAVE_DEBUG_THROWS) -static char assert_buffer[ASSERT_BUFFER_LEN]; +char assert_buffer[ASSERT_BUFFER_LEN]; #endif -#if defined(HAVE_PRINTF) -void assert_print_failed(void) -{ - PRINTF("LEDGER_ASSERT FAILED\n"); -} -#endif - -#if defined(HAVE_LEDGER_ASSERT_DISPLAY) && defined(LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO) +/************************************************************ + * Define behavior when LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO * + ***********************************************************/ +#ifdef LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO +#ifdef HAVE_LEDGER_ASSERT_DISPLAY void assert_display_lr_and_pc(int lr, int pc) { char buff[LR_AND_PC_SIZE]; @@ -54,17 +51,22 @@ void assert_display_lr_and_pc(int lr, int pc) } #endif -#if defined(HAVE_LEDGER_ASSERT_DISPLAY) && defined(LEDGER_ASSERT_CONFIG_MESSAGE_INFO) -void assert_display_message(const char *message) +#ifdef HAVE_PRINTF +void assert_print_lr_and_pc(int lr, int pc) { - char buff[MESSAGE_SIZE]; - - snprintf(buff, MESSAGE_SIZE, "%s\n", message); - strncat(assert_buffer, buff, MESSAGE_SIZE); + lr = compute_address_location(lr); + pc = compute_address_location(pc); + PRINTF("=> LR: 0x%08X \n", lr); + PRINTF("=> PC: 0x%08X \n", pc); } #endif +#endif // LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO -#if defined(HAVE_LEDGER_ASSERT_DISPLAY) && defined(LEDGER_ASSERT_CONFIG_FILE_INFO) +/******************************************************* + * Define behavior when LEDGER_ASSERT_CONFIG_FILE_INFO * + ******************************************************/ +#ifdef LEDGER_ASSERT_CONFIG_FILE_INFO +#ifdef HAVE_LEDGER_ASSERT_DISPLAY void assert_display_file_info(const char *file, unsigned int line) { char buff[FILE_SIZE]; @@ -74,55 +76,45 @@ void assert_display_file_info(const char *file, unsigned int line) } #endif -#if defined(HAVE_PRINTF) && defined(LEDGER_ASSERT_CONFIG_LR_AND_PC_INFO) -void assert_print_lr_and_pc(int lr, int pc) -{ - lr = compute_address_location(lr); - pc = compute_address_location(pc); - PRINTF("LEDGER_ASSERT FAILED\n"); - PRINTF("=> LR: 0x%08X \n", lr); - PRINTF("=> PC: 0x%08X \n", pc); -} -#endif - -#if defined(HAVE_PRINTF) && defined(LEDGER_ASSERT_CONFIG_FILE_INFO) +#ifdef HAVE_PRINTF void assert_print_file_info(const char *file, int line) { PRINTF("%s::%d \n", file, line); } #endif +#endif // LEDGER_ASSERT_CONFIG_FILE_INFO -#if defined(HAVE_PRINTF) && defined(LEDGER_ASSERT_CONFIG_MESSAGE_INFO) -void assert_print_message(const char *message) -{ - if (message) { - PRINTF("%s\n", message); - } -} -#endif - -#if defined(HAVE_DEBUG_THROWS) +/************************************* + * Specific mechanism to debug THROW * + ************************************/ +#ifdef HAVE_DEBUG_THROWS void throw_display_lr(int e, int lr) { lr = compute_address_location(lr); snprintf(assert_buffer, ASSERT_BUFFER_LEN, "e=0x%04X\n LR=0x%08X\n", e, lr); } -#endif -#if defined(HAVE_PRINTF) && defined(HAVE_DEBUG_THROWS) +#ifdef HAVE_PRINTF void throw_print_lr(int e, int lr) { lr = compute_address_location(lr); PRINTF("exception[0x%04X]: LR=0x%08X\n", e, lr); } #endif +#endif // HAVE_DEBUG_THROWS +/******************* + * Common app exit * + ******************/ void assert_exit(bool confirm) { UNUSED(confirm); os_sched_exit(-1); } +/************************************ + * Display info on screen mechanism * + ***********************************/ #if defined(HAVE_LEDGER_ASSERT_DISPLAY) || defined(HAVE_DEBUG_THROWS) #ifdef HAVE_BAGL UX_STEP_CB(ux_error, From ff2eb840b749ea0d7882ee4dec68bd6fa2229542 Mon Sep 17 00:00:00 2001 From: Xavier Chapron Date: Mon, 15 Jan 2024 17:44:07 +0100 Subject: [PATCH 10/13] lib_cxng: Cleanup (cherry picked from commit b8c46e958b7e0e3aac10af5c85a3d26f4318b628) --- lib_cxng/include/lcx_groestl.h | 2 +- lib_cxng/include/lcx_hash.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib_cxng/include/lcx_groestl.h b/lib_cxng/include/lcx_groestl.h index 6fbed8b7f..e7633e132 100644 --- a/lib_cxng/include/lcx_groestl.h +++ b/lib_cxng/include/lcx_groestl.h @@ -65,7 +65,7 @@ size_t cx_groestl_get_output_size(const cx_groestl_t *ctx); /** * @brief Initializes a GROESTL context. * - * @param[out] hash Pointer to the context to init.ialize. + * @param[out] hash Pointer to the context to initialize. * * @param[in] size Length of the digest. * diff --git a/lib_cxng/include/lcx_hash.h b/lib_cxng/include/lcx_hash.h index 0716e1cca..3320a3995 100644 --- a/lib_cxng/include/lcx_hash.h +++ b/lib_cxng/include/lcx_hash.h @@ -143,7 +143,6 @@ size_t cx_hash_get_size(const cx_hash_t *ctx); * - INVALID_PARAMETER * - CX_INVALID_PARAMETER */ -// WARN_UNUSED_RESULT cx_err_t cx_hash_no_throw(cx_hash_t *hash, WARN_UNUSED_RESULT cx_err_t cx_hash_no_throw(cx_hash_t *hash, uint32_t mode, const uint8_t *in, From 8b7c42ee0578e84bdb0e11b169b206563cb0e82e Mon Sep 17 00:00:00 2001 From: Xavier Chapron Date: Fri, 19 Jan 2024 15:21:26 +0100 Subject: [PATCH 11/13] lib_cxng: Add missing WARN_UNUSED_RESULT (cherry picked from commit be5872c7e5a38fb57a83dd35a682a2e07b7d88e8) --- lib_cxng/include/lcx_ecdsa.h | 28 ++++++++++++++-------------- lib_cxng/include/lcx_ecschnorr.h | 28 ++++++++++++++-------------- lib_cxng/include/lcx_eddsa.h | 12 ++++++------ 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/lib_cxng/include/lcx_ecdsa.h b/lib_cxng/include/lcx_ecdsa.h index 0b7f6fd27..4693fec45 100644 --- a/lib_cxng/include/lcx_ecdsa.h +++ b/lib_cxng/include/lcx_ecdsa.h @@ -152,15 +152,15 @@ DEPRECATED static inline size_t cx_ecdsa_sign(const cx_ecfp_private_key_t *pvkey * - CX_EC_INFINITE_POINT * - CX_INVALID_PARAMETER_VALUE */ -cx_err_t cx_ecdsa_sign_rs_no_throw(const cx_ecfp_private_key_t *key, - uint32_t mode, - cx_md_t hashID, - const uint8_t *hash, - size_t hash_len, - size_t rs_len, - uint8_t *sig_r, - uint8_t *sig_s, - uint32_t *info); +WARN_UNUSED_RESULT cx_err_t cx_ecdsa_sign_rs_no_throw(const cx_ecfp_private_key_t *key, + uint32_t mode, + cx_md_t hashID, + const uint8_t *hash, + size_t hash_len, + size_t rs_len, + uint8_t *sig_r, + uint8_t *sig_s, + uint32_t *info); /** * @brief Verifies an ECDSA signature according to ECDSA specification. @@ -180,11 +180,11 @@ cx_err_t cx_ecdsa_sign_rs_no_throw(const cx_ecfp_private_key_t *key, * * @return 1 if the signature is verified, 0 otherwise. */ -bool cx_ecdsa_verify_no_throw(const cx_ecfp_public_key_t *pukey, - const uint8_t *hash, - size_t hash_len, - const uint8_t *sig, - size_t sig_len); +WARN_UNUSED_RESULT bool cx_ecdsa_verify_no_throw(const cx_ecfp_public_key_t *pukey, + const uint8_t *hash, + size_t hash_len, + const uint8_t *sig, + size_t sig_len); /** * @deprecated diff --git a/lib_cxng/include/lcx_ecschnorr.h b/lib_cxng/include/lcx_ecschnorr.h index 719cc45b4..664894dbf 100644 --- a/lib_cxng/include/lcx_ecschnorr.h +++ b/lib_cxng/include/lcx_ecschnorr.h @@ -71,13 +71,13 @@ * - CX_EC_INFINITE_POINT * - CX_INVALID_PARAMETER_VALUE */ -cx_err_t cx_ecschnorr_sign_no_throw(const cx_ecfp_private_key_t *pvkey, - uint32_t mode, - cx_md_t hashID, - const uint8_t *msg, - size_t msg_len, - uint8_t *sig, - size_t *sig_len); +WARN_UNUSED_RESULT cx_err_t cx_ecschnorr_sign_no_throw(const cx_ecfp_private_key_t *pvkey, + uint32_t mode, + cx_md_t hashID, + const uint8_t *msg, + size_t msg_len, + uint8_t *sig, + size_t *sig_len); /** * @deprecated @@ -128,13 +128,13 @@ DEPRECATED static inline size_t cx_ecschnorr_sign(const cx_ecfp_private_key_t *p * * @return 1 if signature is verified, 0 otherwise. */ -bool cx_ecschnorr_verify(const cx_ecfp_public_key_t *pukey, - uint32_t mode, - cx_md_t hashID, - const uint8_t *msg, - size_t msg_len, - const uint8_t *sig, - size_t sig_len); +WARN_UNUSED_RESULT bool cx_ecschnorr_verify(const cx_ecfp_public_key_t *pukey, + uint32_t mode, + cx_md_t hashID, + const uint8_t *msg, + size_t msg_len, + const uint8_t *sig, + size_t sig_len); #endif diff --git a/lib_cxng/include/lcx_eddsa.h b/lib_cxng/include/lcx_eddsa.h index 54f88686c..c03afb483 100644 --- a/lib_cxng/include/lcx_eddsa.h +++ b/lib_cxng/include/lcx_eddsa.h @@ -131,12 +131,12 @@ DEPRECATED static inline size_t cx_eddsa_sign(const cx_ecfp_private_key_t *pvkey * * @return 1 if the signature is verified, otherwise 0. */ -bool cx_eddsa_verify_no_throw(const cx_ecfp_public_key_t *pukey, - cx_md_t hashID, - const uint8_t *hash, - size_t hash_len, - const uint8_t *sig, - size_t sig_len); +WARN_UNUSED_RESULT bool cx_eddsa_verify_no_throw(const cx_ecfp_public_key_t *pukey, + cx_md_t hashID, + const uint8_t *hash, + size_t hash_len, + const uint8_t *sig, + size_t sig_len); /** * @brief Verifies a signature. From 7e6edc7f0454ae2f9ebd1668fab1be8f5e155a32 Mon Sep 17 00:00:00 2001 From: Xavier Chapron Date: Tue, 23 Jan 2024 14:40:20 +0100 Subject: [PATCH 12/13] cx_errors.h: Add CX_ASSERT macro (cherry picked from commit 69c8890dd4565266971f774a58bab58931f7859d) --- include/cx_errors.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/cx_errors.h b/include/cx_errors.h index a2b260612..b409ec899 100644 --- a/include/cx_errors.h +++ b/include/cx_errors.h @@ -6,6 +6,7 @@ #pragma once #include +#include "ledger_assert.h" /** * Checks the error code of a function. @@ -32,6 +33,15 @@ } \ } while (0) +/** + * Checks the error code of a function and assert in case of error. + */ +#define CX_ASSERT(call) \ + do { \ + cx_err_t _assert_err = call; \ + LEDGER_ASSERT(!_assert_err, "err 0x%X <%s>", _assert_err, #call); \ + } while (0) + /** Success. */ #define CX_OK 0x00000000 From c323281093e2a0d63feded4d6f56a26d5c5ac4a2 Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Wed, 24 Jan 2024 11:06:53 +0100 Subject: [PATCH 13/13] Remove currentHeight unused var in nbgl_use_case (cherry picked from commit a9d8362a0dc90f4c4ebd387ebbf8df8ba9858c39) --- lib_nbgl/src/nbgl_use_case.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib_nbgl/src/nbgl_use_case.c b/lib_nbgl/src/nbgl_use_case.c index 923f6bcb2..b3020c5ae 100644 --- a/lib_nbgl/src/nbgl_use_case.c +++ b/lib_nbgl/src/nbgl_use_case.c @@ -776,16 +776,14 @@ uint8_t nbgl_useCaseGetNbTagValuesInPage(uint8_t nbPair */ uint8_t nbgl_useCaseGetNbPagesForTagValueList(const nbgl_layoutTagValueList_t *tagValueList) { - uint8_t nbPages = 0; - uint8_t nbPairs = tagValueList->nbPairs; - uint8_t nbPairsInPage; - uint16_t currentHeight = 0; - uint8_t i = 0; - bool tooLongToFit; + uint8_t nbPages = 0; + uint8_t nbPairs = tagValueList->nbPairs; + uint8_t nbPairsInPage; + uint8_t i = 0; + bool tooLongToFit; while (i < tagValueList->nbPairs) { // upper margin - currentHeight += 24; nbPairsInPage = nbgl_useCaseGetNbTagValuesInPage(nbPairs, tagValueList, i, &tooLongToFit); i += nbPairsInPage; setNbPairs(nbPages, nbPairsInPage, tooLongToFit);