From 5a0ad66bd66bb2a6b64d2955588f90574f15cf67 Mon Sep 17 00:00:00 2001 From: Ilya Artemov Date: Tue, 18 Feb 2025 12:17:27 +0100 Subject: [PATCH 1/2] 1st draft of the app storage functionality simplification (only for Stax so far) --- Makefile.standard_app | 5 +- include/app_storage.h | 56 ++++++-------- lib_standard_app/app_storage.c | 133 +++++++++++++++++++++++---------- lib_standard_app/main.c | 7 ++ target/stax/script.ld | 6 +- 5 files changed, 131 insertions(+), 76 deletions(-) diff --git a/Makefile.standard_app b/Makefile.standard_app index 95c02823a..1758b230a 100644 --- a/Makefile.standard_app +++ b/Makefile.standard_app @@ -1,6 +1,6 @@ #******************************************************************************* # Ledger SDK -# (c) 2022 Ledger +# (c) 2022-2025 Ledger # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -53,6 +53,9 @@ endif ifeq ($(ENABLE_APP_STORAGE), 1) HAVE_APP_STORAGE = 1 DEFINES += HAVE_APP_STORAGE + DEFINES += APP_STORAGE_SIZE=$(APP_STORAGE_SIZE) + DEFINES += HAVE_APP_STORAGE_PROP_SETTINGS=$(ENABLE_APP_STORAGE_PROP_SETTINGS) + DEFINES += HAVE_APP_STORAGE_PROP_DATA=$(ENABLE_APP_STORAGE_PROP_DATA) endif ##################################################################### diff --git a/include/app_storage.h b/include/app_storage.h index 4293890dc..88f297a0c 100644 --- a/include/app_storage.h +++ b/include/app_storage.h @@ -1,5 +1,5 @@ /***************************************************************************** - * (c) 2024 Ledger SAS. + * (c) 2024-2025 Ledger SAS. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,53 +29,43 @@ ///< bit to indicate in properties that the application contains data #define APP_STORAGE_PROP_DATA (1 << 1) +///< Tag length +#define APP_STORAGE_TAG_LEN 4 + +///< Tag value +#define APP_STORAGE_TAG "NVRA" + +///< Header structure version +#define APP_STORAGE_HEADER_STRUCT_VERSION 1 + /** * @brief Structure defining the header of application storage header * */ typedef struct app_storage_header_s { - char tag[4]; ///< ['N','V','R','A'] array, when properly initialized + char tag[APP_STORAGE_TAG_LEN]; ///< ['N','V','R','A'] array, when properly initialized uint32_t size; ///< size in bytes of the data (app_storage_data_t structure) - uint16_t struct_version; ///< version of the structure of data (to be set once at first - ///< application start-up) + uint16_t struct_version; ///< version of this structure (for OS) uint16_t properties; ///< used as a bitfield to set properties, like: contains settings, ///< contains sensitive data uint32_t data_version; ///< version of the content of data (to be updated every time data are ///< updated) } app_storage_header_t; -/** - * @brief Structure defining the application storage - * - */ -typedef struct app_storage_s { - app_storage_header_t header; ///< header describing the data -#ifndef HAVE_BOLOS - app_storage_data_t data; ///< application data, app_storage_data_t must be defined in - ///< "app_storage_data.h" file -#endif // HAVE_BOLOS -} app_storage_t; - #ifndef HAVE_BOLOS -/** - * @brief This variable must be defined in Application code, and never used directly, - * except by @ref N_nvram - * - */ -extern const app_storage_t N_app_storage_real; -/** - * @brief To be used by function accessing application storage data (not N_storage_real) - * - */ -#define N_app_storage (*(volatile app_storage_t *) PIC(&N_app_storage_real)) - -void app_storage_init(uint32_t data_version); +/* Getters for system information */ uint32_t app_storage_get_size(void); -uint16_t app_storage_get_struct_version(void); uint16_t app_storage_get_properties(void); uint32_t app_storage_get_data_version(void); -bool app_storage_is_initalized(void); -void app_storage_set_data_version(uint32_t data_version); -#endif // HAVE_BOLOS +/* Getter for the application data */ +/* XXX: It is more practical finally to give the direct read access to the NVM for the app */ +const void *app_storage_get(void); + +/* Setters */ +int32_t app_storage_pwrite(const void *buf, uint32_t nbyte, uint32_t offset); +void app_storage_set_data_version(uint32_t data_version); +void app_storage_increment_data_version(void); + +#endif // #ifndef HAVE_BOLOS diff --git a/lib_standard_app/app_storage.c b/lib_standard_app/app_storage.c index c8ad9ddc6..22187699f 100644 --- a/lib_standard_app/app_storage.c +++ b/lib_standard_app/app_storage.c @@ -1,5 +1,5 @@ /***************************************************************************** - * (c) 2024 Ledger SAS. + * (c) 2024-2025 Ledger SAS. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,87 +20,138 @@ #include "os_nvm.h" #include "os_pic.h" -// TODO: Create new section for the linker script -/*app_storage app_storage_real __attribute__((section(".bss.app_storage")));*/ -const app_storage_t N_app_storage_real; +/* The storage consists of the system and the app parts */ +const uint8_t app_storage_real[sizeof(app_storage_header_t) + APP_STORAGE_SIZE] + __attribute__((section(".storage_section"))); +#define app_storage_pic ((volatile uint8_t *) PIC(&app_storage_real)) /** - * @brief init header of application storage structure : - * - set "NVRA" tag - * - set size - * - set struct and data versions - * - set properties - * @param data_version Version of the data + * @brief checks if the app storage struct is initialized */ -void app_storage_init(uint32_t data_version) +static bool app_storage_is_initalized(void) { + if (memcmp((void *) &((app_storage_header_t *) app_storage_pic)->tag, + APP_STORAGE_TAG, + APP_STORAGE_TAG_LEN)) { + return false; + } + if (((app_storage_header_t *) app_storage_pic)->size == 0) { + return false; + } + return true; +} + +/** + * @brief initializes the header of application storage structure: + * - checks if it already initialized, if not: + * - sets "NVRA" tag + * - sets initial size (0) + * - sets struct and data versions (1) + * - sets properties (from Makefile) + */ +void app_storage_init(void) +{ + if (app_storage_is_initalized()) { + return; + } + app_storage_header_t header = {0}; - memcpy(header.tag, (void *) "NVRA", 4); + // TODO: should we erase all the area ? + // In any case erase must be page aligned for the length and on the app side we do not know the + // page size nvm_erase((void *) app_storage_pic, sizeof(app_storage_header_t) + + // APP_STORAGE_SIZE); + + memcpy(header.tag, (void *) APP_STORAGE_TAG, APP_STORAGE_TAG_LEN); // APP_STORAGE_DATA_STRUCT_VERSION and APP_STORAGE_PROPERTIES must be defined in // app_storage_data.h - header.struct_version = APP_STORAGE_DATA_STRUCT_VERSION; - header.data_version = data_version; - header.properties = APP_STORAGE_PROPERTIES; - // TODO: Doing this lead to have app storage bigger than needed - header.size = sizeof(app_storage_data_t); - nvm_write((void *) &N_app_storage.header, &header, sizeof(header)); + header.struct_version = APP_STORAGE_HEADER_STRUCT_VERSION; + header.data_version = 1; + header.properties = ((HAVE_APP_STORAGE_PROP_SETTINGS << 0) | (HAVE_APP_STORAGE_PROP_DATA << 1)); + header.size = 0; + nvm_write((void *) app_storage_pic, &header, sizeof(header)); } /** - * @brief get the size of app data + * @brief returns the size of app data */ uint32_t app_storage_get_size(void) { - return N_app_storage.header.size; + return ((app_storage_header_t *) app_storage_pic)->size; } /** - * @brief get the version of app data structure + * @brief returns the version of app data */ -uint16_t app_storage_get_struct_version(void) +uint32_t app_storage_get_data_version(void) { - return N_app_storage.header.struct_version; + return ((app_storage_header_t *) app_storage_pic)->data_version; } /** - * @brief get the version of app data + * @brief returns the properties of app data */ -uint32_t app_storage_get_data_version(void) +uint16_t app_storage_get_properties(void) { - return N_app_storage.header.data_version; + return ((app_storage_header_t *) app_storage_pic)->properties; } /** - * @brief get the properties of app data + * @brief increments by 1 the data_version field */ -uint16_t app_storage_get_properties(void) +void app_storage_increment_data_version(void) { - return N_app_storage.header.properties; + uint32_t data_version = ((app_storage_header_t *) app_storage_pic)->data_version; + data_version++; + nvm_write((void *) &app_storage_pic[offsetof(app_storage_header_t, data_version)], + (void *) &data_version, + sizeof(data_version)); } /** - * @brief ensure app storage struct is initialized + * @brief sets the data_version field */ -bool app_storage_is_initalized(void) +void app_storage_set_data_version(uint32_t data_version) { - if (memcmp((void *) N_app_storage.header.tag, "NVRA", 4)) { - return false; + nvm_write((void *) &app_storage_pic[offsetof(app_storage_header_t, data_version)], + (void *) &data_version, + sizeof(((app_storage_header_t *) app_storage_pic)->tag)); +} + +/** + * @brief writes application storage data with length and offset + */ +int32_t app_storage_pwrite(const void *buf, uint32_t nbyte, uint32_t offset) +{ + /* Input parameters verification */ + if (buf == NULL) { + return -1; } - if (N_app_storage.header.size == 0) { - return false; + + uint32_t size = offset + nbyte; + if (size >= APP_STORAGE_SIZE) { + return -1; } - return true; + + /* Updating data */ + nvm_write( + (void *) &app_storage_pic[sizeof(app_storage_header_t) + offset], (void *) buf, nbyte); + + /* Updating size if it increased */ + if (((app_storage_header_t *) app_storage_pic)->size < size) { + nvm_write((void *) &app_storage_pic[offsetof(app_storage_header_t, size)], + (void *) &size, + sizeof(((app_storage_header_t *) app_storage_pic)->size)); + } + return nbyte; } /** - * @brief set data version of app data + * @brief returns the base address of the application storage */ -void app_storage_set_data_version(uint32_t data_version) +const void *app_storage_get(void) { - nvm_write((void *) &N_app_storage.header.data_version, - (void *) &data_version, - sizeof(N_app_storage.header.data_version)); + return (const void *) &app_storage_pic[sizeof(app_storage_header_t)]; } #endif // HAVE_APP_STORAGE diff --git a/lib_standard_app/main.c b/lib_standard_app/main.c index fc0b0fbb5..56d5fe68e 100644 --- a/lib_standard_app/main.c +++ b/lib_standard_app/main.c @@ -40,6 +40,8 @@ WEAK void __attribute__((noreturn)) app_exit(void) os_sched_exit(-1); } +extern void app_storage_init(void); + WEAK void common_app_init(void) { UX_INIT(); @@ -55,6 +57,11 @@ WEAK void common_app_init(void) BLE_power(0, NULL); BLE_power(1, NULL); #endif // HAVE_BLE + +#ifdef HAVE_APP_STORAGE + /* Implicit app storage initialization */ + app_storage_init(); +#endif // #ifdef HAVE_APP_STORAGE } WEAK void standalone_app_main(void) diff --git a/target/stax/script.ld b/target/stax/script.ld index 4114636c8..ca53b962e 100644 --- a/target/stax/script.ld +++ b/target/stax/script.ld @@ -1,6 +1,6 @@ /******************************************************************************* * Ledger - Secure firmware -* (c) 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023 Ledger +* (c) 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023, 2025 Ledger * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -69,7 +69,11 @@ SECTIONS _nvram_data = .; + /* App storage */ + *(.storage_section) + /* NVM data (ex-filesystem) */ + /* TODO: should we align the start of N_ variables on page size ? */ *(.bss.N_* .rodata.N_*) . = ALIGN(PAGE_SIZE); From 6654e6b4f0517b6170e0ba0c779b15ef3c22e0f3 Mon Sep 17 00:00:00 2001 From: Ilya Artemov Date: Thu, 20 Feb 2025 15:09:43 +0100 Subject: [PATCH 2/2] Addressing ledger-secure-sdk/pull/865 comments --- Makefile.standard_app | 4 ++ include/app_storage.h | 15 +++---- lib_standard_app/app_storage.c | 74 +++++++++++++++++----------------- target/flex/script.ld | 5 ++- target/nanos2/script.ld | 5 ++- target/nanox/script.ld | 5 ++- 6 files changed, 60 insertions(+), 48 deletions(-) diff --git a/Makefile.standard_app b/Makefile.standard_app index 1758b230a..a5f7a9f27 100644 --- a/Makefile.standard_app +++ b/Makefile.standard_app @@ -53,6 +53,10 @@ endif ifeq ($(ENABLE_APP_STORAGE), 1) HAVE_APP_STORAGE = 1 DEFINES += HAVE_APP_STORAGE + ifeq ($(APP_STORAGE_SIZE),) + # Fall back to maximum page size for all the devices + APP_STORAGE_SIZE := 480 # 512 - 32 bytes for the system header + endif DEFINES += APP_STORAGE_SIZE=$(APP_STORAGE_SIZE) DEFINES += HAVE_APP_STORAGE_PROP_SETTINGS=$(ENABLE_APP_STORAGE_PROP_SETTINGS) DEFINES += HAVE_APP_STORAGE_PROP_DATA=$(ENABLE_APP_STORAGE_PROP_DATA) diff --git a/include/app_storage.h b/include/app_storage.h index 88f297a0c..ddf093e5f 100644 --- a/include/app_storage.h +++ b/include/app_storage.h @@ -42,7 +42,7 @@ * @brief Structure defining the header of application storage header * */ -typedef struct app_storage_header_s { +typedef struct __attribute__((packed)) app_storage_header_s { char tag[APP_STORAGE_TAG_LEN]; ///< ['N','V','R','A'] array, when properly initialized uint32_t size; ///< size in bytes of the data (app_storage_data_t structure) uint16_t struct_version; ///< version of this structure (for OS) @@ -59,13 +59,14 @@ uint32_t app_storage_get_size(void); uint16_t app_storage_get_properties(void); uint32_t app_storage_get_data_version(void); -/* Getter for the application data */ -/* XXX: It is more practical finally to give the direct read access to the NVM for the app */ -const void *app_storage_get(void); +/* Reads app storage data */ +int32_t app_storage_pread(void *buf, uint32_t nbyte, uint32_t offset); -/* Setters */ +/* Writes app storage data */ int32_t app_storage_pwrite(const void *buf, uint32_t nbyte, uint32_t offset); -void app_storage_set_data_version(uint32_t data_version); -void app_storage_increment_data_version(void); + +/* Setters */ +void app_storage_set_data_version(uint32_t data_version); +void app_storage_increment_data_version(void); #endif // #ifndef HAVE_BOLOS diff --git a/lib_standard_app/app_storage.c b/lib_standard_app/app_storage.c index 22187699f..5d77ac062 100644 --- a/lib_standard_app/app_storage.c +++ b/lib_standard_app/app_storage.c @@ -21,21 +21,20 @@ #include "os_pic.h" /* The storage consists of the system and the app parts */ -const uint8_t app_storage_real[sizeof(app_storage_header_t) + APP_STORAGE_SIZE] - __attribute__((section(".storage_section"))); -#define app_storage_pic ((volatile uint8_t *) PIC(&app_storage_real)) +typedef struct __attribute__((packed)) app_storage_s { + app_storage_header_t header; + uint8_t data[APP_STORAGE_SIZE]; +} app_storage_t; + +const app_storage_t app_storage_real __attribute__((section(".storage_section"))); +#define as (*(volatile app_storage_t *) PIC(&app_storage_real)) /** * @brief checks if the app storage struct is initialized */ static bool app_storage_is_initalized(void) { - if (memcmp((void *) &((app_storage_header_t *) app_storage_pic)->tag, - APP_STORAGE_TAG, - APP_STORAGE_TAG_LEN)) { - return false; - } - if (((app_storage_header_t *) app_storage_pic)->size == 0) { + if (memcmp((const void *) &as.header.tag, APP_STORAGE_TAG, APP_STORAGE_TAG_LEN)) { return false; } return true; @@ -57,19 +56,12 @@ void app_storage_init(void) app_storage_header_t header = {0}; - // TODO: should we erase all the area ? - // In any case erase must be page aligned for the length and on the app side we do not know the - // page size nvm_erase((void *) app_storage_pic, sizeof(app_storage_header_t) + - // APP_STORAGE_SIZE); - - memcpy(header.tag, (void *) APP_STORAGE_TAG, APP_STORAGE_TAG_LEN); - // APP_STORAGE_DATA_STRUCT_VERSION and APP_STORAGE_PROPERTIES must be defined in - // app_storage_data.h + memcpy(&header.tag, (void *) APP_STORAGE_TAG, APP_STORAGE_TAG_LEN); header.struct_version = APP_STORAGE_HEADER_STRUCT_VERSION; header.data_version = 1; header.properties = ((HAVE_APP_STORAGE_PROP_SETTINGS << 0) | (HAVE_APP_STORAGE_PROP_DATA << 1)); header.size = 0; - nvm_write((void *) app_storage_pic, &header, sizeof(header)); + nvm_write((void *) &as.header, &header, sizeof(header)); } /** @@ -77,7 +69,7 @@ void app_storage_init(void) */ uint32_t app_storage_get_size(void) { - return ((app_storage_header_t *) app_storage_pic)->size; + return as.header.size; } /** @@ -85,7 +77,7 @@ uint32_t app_storage_get_size(void) */ uint32_t app_storage_get_data_version(void) { - return ((app_storage_header_t *) app_storage_pic)->data_version; + return as.header.data_version; } /** @@ -93,7 +85,7 @@ uint32_t app_storage_get_data_version(void) */ uint16_t app_storage_get_properties(void) { - return ((app_storage_header_t *) app_storage_pic)->properties; + return as.header.properties; } /** @@ -101,21 +93,17 @@ uint16_t app_storage_get_properties(void) */ void app_storage_increment_data_version(void) { - uint32_t data_version = ((app_storage_header_t *) app_storage_pic)->data_version; + uint32_t data_version = as.header.data_version; data_version++; - nvm_write((void *) &app_storage_pic[offsetof(app_storage_header_t, data_version)], - (void *) &data_version, - sizeof(data_version)); + nvm_write((void *) &as.header.data_version, (void *) &data_version, sizeof(data_version)); } /** - * @brief sets the data_version field + * @brief writes application data to the storage */ void app_storage_set_data_version(uint32_t data_version) { - nvm_write((void *) &app_storage_pic[offsetof(app_storage_header_t, data_version)], - (void *) &data_version, - sizeof(((app_storage_header_t *) app_storage_pic)->tag)); + nvm_write((void *) &as.header.data_version, (void *) &data_version, sizeof(data_version)); } /** @@ -134,24 +122,34 @@ int32_t app_storage_pwrite(const void *buf, uint32_t nbyte, uint32_t offset) } /* Updating data */ - nvm_write( - (void *) &app_storage_pic[sizeof(app_storage_header_t) + offset], (void *) buf, nbyte); + nvm_write((void *) &as.data[offset], (void *) buf, nbyte); /* Updating size if it increased */ - if (((app_storage_header_t *) app_storage_pic)->size < size) { - nvm_write((void *) &app_storage_pic[offsetof(app_storage_header_t, size)], - (void *) &size, - sizeof(((app_storage_header_t *) app_storage_pic)->size)); + if (as.header.size < size) { + nvm_write((void *) &as.header.size, (void *) &size, sizeof(size)); } return nbyte; } /** - * @brief returns the base address of the application storage + * @brief reads application data from the storage */ -const void *app_storage_get(void) +int32_t app_storage_pread(void *buf, uint32_t nbyte, uint32_t offset) { - return (const void *) &app_storage_pic[sizeof(app_storage_header_t)]; + /* Input parameters verification */ + if (buf == NULL) { + return -1; + } + + uint32_t size = offset + nbyte; + if (size >= as.header.size) { + return -1; + } + + /* Reading data */ + memcpy((void *) buf, (void *) &as.data[offset], nbyte); + + return nbyte; } #endif // HAVE_APP_STORAGE diff --git a/target/flex/script.ld b/target/flex/script.ld index 4114636c8..22d1b472b 100644 --- a/target/flex/script.ld +++ b/target/flex/script.ld @@ -1,6 +1,6 @@ /******************************************************************************* * Ledger - Secure firmware -* (c) 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023 Ledger +* (c) 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023, 2025 Ledger * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -69,6 +69,9 @@ SECTIONS _nvram_data = .; + /* App storage */ + *(.storage_section) + /* NVM data (ex-filesystem) */ *(.bss.N_* .rodata.N_*) diff --git a/target/nanos2/script.ld b/target/nanos2/script.ld index 94b0b2e18..1f53fe286 100644 --- a/target/nanos2/script.ld +++ b/target/nanos2/script.ld @@ -1,6 +1,6 @@ /******************************************************************************* * Ledger - Secure firmware -* (c) 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023 Ledger +* (c) 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023, 2025 Ledger * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -68,6 +68,9 @@ SECTIONS _nvram_data = .; + /* App storage */ + *(.storage_section) + /* NVM data (ex-filesystem) */ *(.bss.N_* .rodata.N_*) diff --git a/target/nanox/script.ld b/target/nanox/script.ld index cfca329f1..b24cf95d2 100644 --- a/target/nanox/script.ld +++ b/target/nanox/script.ld @@ -1,6 +1,6 @@ /******************************************************************************* * Ledger - Secure firmware -* (c) 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023 Ledger +* (c) 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023, 2025 Ledger * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -70,6 +70,9 @@ SECTIONS _nvram_data = .; + /* App storage */ + *(.storage_section) + /* NVM data (ex-filesystem) */ *(.bss.N_* .rodata.N_*)