1st draft of the app storage functionality simplification (only for S…

…tax so far)

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
 
 #####################################################################

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

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

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)

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);