From 60bb1bf9b59928023deb38a1b32aa6571ec6c5a4 Mon Sep 17 00:00:00 2001 From: Tomi Fontanilles Date: Mon, 11 Nov 2024 18:04:38 +0200 Subject: [PATCH 1/7] secure_storage: add a ZMS-based implementation of the ITS store module It becomes the new default when the secure_storage_its_partition devicetree chosen property is defined as it is a preferred alternative. See the help message of the `CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS` Kconfig option for more information. Signed-off-by: Tomi Fontanilles --- subsys/secure_storage/Kconfig.its_store | 33 ++++- .../zephyr/secure_storage/its/common.h | 1 + .../zephyr/secure_storage/its/store.h | 8 +- subsys/secure_storage/src/its/CMakeLists.txt | 5 +- .../{store_settings.c => store/settings.c} | 0 subsys/secure_storage/src/its/store/zms.c | 122 ++++++++++++++++++ .../subsys/secure_storage/psa/its/src/main.c | 2 +- 7 files changed, 162 insertions(+), 9 deletions(-) rename subsys/secure_storage/src/its/{store_settings.c => store/settings.c} (100%) create mode 100644 subsys/secure_storage/src/its/store/zms.c diff --git a/subsys/secure_storage/Kconfig.its_store b/subsys/secure_storage/Kconfig.its_store index 9e4d9b6502059..99ac19e375737 100644 --- a/subsys/secure_storage/Kconfig.its_store +++ b/subsys/secure_storage/Kconfig.its_store @@ -4,9 +4,26 @@ choice SECURE_STORAGE_ITS_STORE_IMPLEMENTATION prompt "ITS store module implementation" +DT_ITS_PARTITION := $(dt_chosen_path,secure_storage_its_partition) +DT_STORAGE_PARTITION := $(dt_nodelabel_path,storage_partition) + +config SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS + bool "ITS store module implementation using ZMS for storage" + depends on FLASH_HAS_DRIVER_ENABLED \ + && $(dt_path_enabled,$(DT_ITS_PARTITION)) \ + && $(dt_node_has_compat,$(dt_node_parent,$(DT_ITS_PARTITION)),fixed-partitions) + select ZMS + help + This implementation of the ITS store module makes direct use of ZMS for storage. + It needs a `secure_storage_its_partition` devicetree chosen property that points + to a fixed storage partition that will be dedicated to the ITS. It has lower + overhead compared to the settings-based implementation, both in terms of runtime + execution and storage space, and also ROM footprint if the settings subsystem is disabled. + As this implementations directly maps the PSA storage UIDs to ZMS entry IDs, it limits + their values to the first 30 bits. + config SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_SETTINGS bool "ITS store module implementation using the settings subsystem for storage" - DT_STORAGE_PARTITION := $(dt_nodelabel_path,storage_partition) depends on FLASH_HAS_DRIVER_ENABLED \ && $(dt_path_enabled,$(DT_STORAGE_PARTITION)) \ && $(dt_node_has_compat,$(dt_node_parent,$(DT_STORAGE_PARTITION)),fixed-partitions) @@ -25,6 +42,20 @@ config SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_CUSTOM endchoice # SECURE_STORAGE_ITS_STORE_IMPLEMENTATION +if SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS + +config SECURE_STORAGE_ITS_STORE_ZMS_SECTOR_SIZE + int "Sector size of the ZMS partition" + default 4096 + help + The sector size impacts the runtime behavior of ZMS and restricts the maximum + ITS entry data size (which is the sector size minus ZMS and ITS overhead). + Changing it will result in loss of existing data stored on a partition. + It must be a multiple of the flash page size on devices that require an erase. + See the ZMS documentation for more information. + +endif # SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS + if SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_SETTINGS config SECURE_STORAGE_ITS_STORE_SETTINGS_PREFIX diff --git a/subsys/secure_storage/include/internal/zephyr/secure_storage/its/common.h b/subsys/secure_storage/include/internal/zephyr/secure_storage/its/common.h index 4b73eafcf3d88..347d96f435e8e 100644 --- a/subsys/secure_storage/include/internal/zephyr/secure_storage/its/common.h +++ b/subsys/secure_storage/include/internal/zephyr/secure_storage/its/common.h @@ -19,6 +19,7 @@ typedef enum { SECURE_STORAGE_ITS_CALLER_PSA_ITS, SECURE_STORAGE_ITS_CALLER_PSA_PS, SECURE_STORAGE_ITS_CALLER_MBEDTLS, + SECURE_STORAGE_ITS_CALLER_COUNT } secure_storage_its_caller_id_t; /** The UID (caller + entry IDs) of an ITS entry. */ diff --git a/subsys/secure_storage/include/internal/zephyr/secure_storage/its/store.h b/subsys/secure_storage/include/internal/zephyr/secure_storage/its/store.h index fec3448b21589..6dbb7a8a3be07 100644 --- a/subsys/secure_storage/include/internal/zephyr/secure_storage/its/store.h +++ b/subsys/secure_storage/include/internal/zephyr/secure_storage/its/store.h @@ -19,9 +19,7 @@ * @param data_length The number of bytes in `data`. * @param data The data to store. * - * @retval `PSA_SUCCESS` on success. - * @retval `PSA_ERROR_INSUFFICIENT_STORAGE` if there is insufficient storage space. - * @retval `PSA_ERROR_STORAGE_FAILURE` on any other failure. + * @return One of the return values of `psa_its_set()`. */ psa_status_t secure_storage_its_store_set(secure_storage_its_uid_t uid, size_t data_length, const void *data); @@ -34,9 +32,7 @@ psa_status_t secure_storage_its_store_set(secure_storage_its_uid_t uid, * @param[out] data_length On success, the number of bytes written to `data`. * May be less than `data_size`. * - * @retval `PSA_SUCCESS` on success. - * @retval `PSA_ERROR_DOES_NOT_EXIST` if no entry with the given UID exists. - * @retval `PSA_ERROR_STORAGE_FAILURE` on any other failure. + * @return One of the return values of `psa_its_get()`. */ psa_status_t secure_storage_its_store_get(secure_storage_its_uid_t uid, size_t data_size, void *data, size_t *data_length); diff --git a/subsys/secure_storage/src/its/CMakeLists.txt b/subsys/secure_storage/src/its/CMakeLists.txt index b9e2bb877d9c1..90340047ff5e3 100644 --- a/subsys/secure_storage/src/its/CMakeLists.txt +++ b/subsys/secure_storage/src/its/CMakeLists.txt @@ -24,6 +24,9 @@ if (NOT CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_NO_INSECURE_KEY_WARNING) endif() endif() +zephyr_library_sources_ifdef(CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS + store/zms.c +) zephyr_library_sources_ifdef(CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_SETTINGS - store_settings.c + store/settings.c ) diff --git a/subsys/secure_storage/src/its/store_settings.c b/subsys/secure_storage/src/its/store/settings.c similarity index 100% rename from subsys/secure_storage/src/its/store_settings.c rename to subsys/secure_storage/src/its/store/settings.c diff --git a/subsys/secure_storage/src/its/store/zms.c b/subsys/secure_storage/src/its/store/zms.c new file mode 100644 index 0000000000000..a0703e81e72fc --- /dev/null +++ b/subsys/secure_storage/src/its/store/zms.c @@ -0,0 +1,122 @@ +/* Copyright (c) 2024 Nordic Semiconductor + * SPDX-License-Identifier: Apache-2.0 + */ +#include +#include +#include +#include +#ifdef CONFIG_SECURE_STORAGE_ITS_IMPLEMENTATION_ZEPHYR +#include +#endif + +LOG_MODULE_DECLARE(secure_storage, CONFIG_SECURE_STORAGE_LOG_LEVEL); + +BUILD_ASSERT(CONFIG_SECURE_STORAGE_ITS_STORE_ZMS_SECTOR_SIZE + > 2 * CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE); + +#define PARTITION_DT_NODE DT_CHOSEN(secure_storage_its_partition) + +static struct zms_fs s_zms = { + .flash_device = FIXED_PARTITION_NODE_DEVICE(PARTITION_DT_NODE), + .offset = FIXED_PARTITION_NODE_OFFSET(PARTITION_DT_NODE), + .sector_size = CONFIG_SECURE_STORAGE_ITS_STORE_ZMS_SECTOR_SIZE, +}; + +static int init_zms(void) +{ + int ret; + + s_zms.sector_count = FIXED_PARTITION_NODE_SIZE(PARTITION_DT_NODE) / s_zms.sector_size; + + ret = zms_mount(&s_zms); + if (ret) { + LOG_DBG("Failed. (%d)", ret); + } + return ret; +} +SYS_INIT(init_zms, APPLICATION, CONFIG_APPLICATION_INIT_PRIORITY); + +/* Bit position of the ITS caller ID in the ZMS entry ID. */ +#define ITS_CALLER_ID_POS 30 +/* Make sure that every ITS caller ID fits in ZMS entry IDs at the defined position. */ +BUILD_ASSERT(1 << (32 - ITS_CALLER_ID_POS) >= SECURE_STORAGE_ITS_CALLER_COUNT); + +static bool has_forbidden_bits_set(secure_storage_its_uid_t uid) +{ + if (uid.uid & GENMASK64(63, ITS_CALLER_ID_POS)) { + LOG_DBG("UID %u/0x%llx cannot be used as it has bits set past " + "the first " STRINGIFY(ITS_CALLER_ID_POS) " ones.", + uid.caller_id, (unsigned long long)uid.uid); + return true; + } + return false; +} + +static uint32_t zms_id_from(secure_storage_its_uid_t uid) +{ + return (uint32_t)uid.uid | (uid.caller_id << ITS_CALLER_ID_POS); +} + +psa_status_t secure_storage_its_store_set(secure_storage_its_uid_t uid, + size_t data_length, const void *data) +{ + psa_status_t psa_ret; + ssize_t zms_ret; + const uint32_t zms_id = zms_id_from(uid); + + if (has_forbidden_bits_set(uid)) { + return PSA_ERROR_INVALID_ARGUMENT; + } + + zms_ret = zms_write(&s_zms, zms_id, data, data_length); + if (zms_ret == data_length) { + psa_ret = PSA_SUCCESS; + } else if (zms_ret == -ENOSPC) { + psa_ret = PSA_ERROR_INSUFFICIENT_STORAGE; + } else { + psa_ret = PSA_ERROR_STORAGE_FAILURE; + } + LOG_DBG("%s 0x%x with %zu bytes. (%zd)", (psa_ret == PSA_SUCCESS) ? + "Wrote" : "Failed to write", zms_id, data_length, zms_ret); + return psa_ret; +} + +psa_status_t secure_storage_its_store_get(secure_storage_its_uid_t uid, size_t data_size, + void *data, size_t *data_length) +{ + psa_status_t psa_ret; + ssize_t zms_ret; + const uint32_t zms_id = zms_id_from(uid); + + if (has_forbidden_bits_set(uid)) { + return PSA_ERROR_INVALID_ARGUMENT; + } + + zms_ret = zms_read(&s_zms, zms_id, data, data_size); + if (zms_ret > 0) { + *data_length = zms_ret; + psa_ret = PSA_SUCCESS; + } else if (zms_ret == -ENOENT) { + psa_ret = PSA_ERROR_DOES_NOT_EXIST; + } else { + psa_ret = PSA_ERROR_STORAGE_FAILURE; + } + LOG_DBG("%s 0x%x for up to %zu bytes. (%zd)", (psa_ret != PSA_ERROR_STORAGE_FAILURE) ? + "Read" : "Failed to read", zms_id, data_size, zms_ret); + return psa_ret; +} + +psa_status_t secure_storage_its_store_remove(secure_storage_its_uid_t uid) +{ + int zms_ret; + const uint32_t zms_id = zms_id_from(uid); + + if (has_forbidden_bits_set(uid)) { + return PSA_ERROR_INVALID_ARGUMENT; + } + + zms_ret = zms_delete(&s_zms, zms_id); + LOG_DBG("%s 0x%x. (%d)", zms_ret ? "Failed to delete" : "Deleted", zms_id, zms_ret); + BUILD_ASSERT(PSA_SUCCESS == 0); + return zms_ret; +} diff --git a/tests/subsys/secure_storage/psa/its/src/main.c b/tests/subsys/secure_storage/psa/its/src/main.c index d9bab46ba090d..1fb1b522133eb 100644 --- a/tests/subsys/secure_storage/psa/its/src/main.c +++ b/tests/subsys/secure_storage/psa/its/src/main.c @@ -114,7 +114,7 @@ ZTEST(secure_storage_psa_its, test_write_once_flag) { psa_status_t ret; /* Use a UID that isn't used in the other tests for the write-once entry. */ - const psa_storage_uid_t uid = ~UID; + const psa_storage_uid_t uid = 1 << 16; const uint8_t data[MAX_DATA_SIZE] = {}; struct psa_storage_info_t info; From f8dbc3158e4b051e24f6f9183a799d8ba0e22cc4 Mon Sep 17 00:00:00 2001 From: Tomi Fontanilles Date: Tue, 12 Nov 2024 13:29:01 +0200 Subject: [PATCH 2/7] tests: secure_storage: psa: its: add a test for the ZMS ITS store module Add a test scenario for the ITS store module implementation that uses ZMS for storage. Reorganize the others at the same time. Signed-off-by: Tomi Fontanilles --- .../psa/its/overlay-default_store.conf | 2 - ...m_store.conf => overlay-store_custom.conf} | 0 .../psa/its/overlay-store_settings.conf | 7 +++ ...orm.conf => overlay-transform_custom.conf} | 3 +- ...rm.conf => overlay-transform_default.conf} | 3 +- .../secure_storage/psa/its/testcase.yaml | 43 ++++++++++--------- .../subsys/secure_storage/psa/its/zms.overlay | 5 +++ 7 files changed, 37 insertions(+), 26 deletions(-) delete mode 100644 tests/subsys/secure_storage/psa/its/overlay-default_store.conf rename tests/subsys/secure_storage/psa/its/{overlay-custom_store.conf => overlay-store_custom.conf} (100%) create mode 100644 tests/subsys/secure_storage/psa/its/overlay-store_settings.conf rename tests/subsys/secure_storage/psa/its/{overlay-custom_transform.conf => overlay-transform_custom.conf} (57%) rename tests/subsys/secure_storage/psa/its/{overlay-default_transform.conf => overlay-transform_default.conf} (60%) create mode 100644 tests/subsys/secure_storage/psa/its/zms.overlay diff --git a/tests/subsys/secure_storage/psa/its/overlay-default_store.conf b/tests/subsys/secure_storage/psa/its/overlay-default_store.conf deleted file mode 100644 index 584a2d08febb5..0000000000000 --- a/tests/subsys/secure_storage/psa/its/overlay-default_store.conf +++ /dev/null @@ -1,2 +0,0 @@ -# Limit the space available for the maximum entry test to not take too long. -CONFIG_SETTINGS_NVS_SECTOR_COUNT=2 diff --git a/tests/subsys/secure_storage/psa/its/overlay-custom_store.conf b/tests/subsys/secure_storage/psa/its/overlay-store_custom.conf similarity index 100% rename from tests/subsys/secure_storage/psa/its/overlay-custom_store.conf rename to tests/subsys/secure_storage/psa/its/overlay-store_custom.conf diff --git a/tests/subsys/secure_storage/psa/its/overlay-store_settings.conf b/tests/subsys/secure_storage/psa/its/overlay-store_settings.conf new file mode 100644 index 0000000000000..e6604bc94b011 --- /dev/null +++ b/tests/subsys/secure_storage/psa/its/overlay-store_settings.conf @@ -0,0 +1,7 @@ +CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_SETTINGS=y + +# 256 - flags (1) - CONFIG_SECURE_STORAGE_ITS_TRANSFORM_OUTPUT_OVERHEAD (28) +CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE=227 + +# Limit the space available for the maximum entry test to not take too long with NVS. +CONFIG_SETTINGS_NVS_SECTOR_COUNT=2 diff --git a/tests/subsys/secure_storage/psa/its/overlay-custom_transform.conf b/tests/subsys/secure_storage/psa/its/overlay-transform_custom.conf similarity index 57% rename from tests/subsys/secure_storage/psa/its/overlay-custom_transform.conf rename to tests/subsys/secure_storage/psa/its/overlay-transform_custom.conf index 8a95663d98a7f..2d845b7321b07 100644 --- a/tests/subsys/secure_storage/psa/its/overlay-custom_transform.conf +++ b/tests/subsys/secure_storage/psa/its/overlay-transform_custom.conf @@ -1,5 +1,4 @@ CONFIG_SECURE_STORAGE_ITS_TRANSFORM_IMPLEMENTATION_CUSTOM=y CONFIG_SECURE_STORAGE_ITS_TRANSFORM_OUTPUT_OVERHEAD=0 -# SETTINGS_MAX_VAL_LEN (256) - flags (1) -CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE=255 +CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE=500 diff --git a/tests/subsys/secure_storage/psa/its/overlay-default_transform.conf b/tests/subsys/secure_storage/psa/its/overlay-transform_default.conf similarity index 60% rename from tests/subsys/secure_storage/psa/its/overlay-default_transform.conf rename to tests/subsys/secure_storage/psa/its/overlay-transform_default.conf index dd30410730963..c743a735e4d56 100644 --- a/tests/subsys/secure_storage/psa/its/overlay-default_transform.conf +++ b/tests/subsys/secure_storage/psa/its/overlay-transform_default.conf @@ -5,5 +5,4 @@ CONFIG_TIMER_RANDOM_GENERATOR=y CONFIG_COMMON_LIBC_MALLOC_ARENA_SIZE=2048 CONFIG_MBEDTLS_PSA_CRYPTO_C=y -# SETTINGS_MAX_VAL_LEN (256) - flags (1) - CONFIG_SECURE_STORAGE_ITS_TRANSFORM_OUTPUT_OVERHEAD (28) -CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE=227 +CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE=300 diff --git a/tests/subsys/secure_storage/psa/its/testcase.yaml b/tests/subsys/secure_storage/psa/its/testcase.yaml index c59c58532b493..b3dce34dd1842 100644 --- a/tests/subsys/secure_storage/psa/its/testcase.yaml +++ b/tests/subsys/secure_storage/psa/its/testcase.yaml @@ -1,42 +1,45 @@ common: integration_platforms: - native_sim + - nrf54l15dk/nrf54l15/cpuapp platform_exclude: - qemu_cortex_m0 # settings subsystem initialization fails timeout: 600 tags: - psa.secure_storage tests: - secure_storage.psa.its.secure_storage: - filter: CONFIG_SECURE_STORAGE and not CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_NONE + secure_storage.psa.its.secure_storage.zms: + # DT-based filtering is not possible for this test scenario. + # Platforms with a storage_partition must be manually added here. + platform_allow: native_sim mps2/an385 qemu_x86/atom qemu_x86_64/atom + nrf54l15dk/nrf54l15/cpuapp nrf5340dk/nrf5340/cpuapp nrf52840dk/nrf52840 + nrf9151dk/nrf9151 nrf9160dk/nrf9160 nrf9161dk/nrf9161 + extra_args: + - EXTRA_DTC_OVERLAY_FILE=zms.overlay + - EXTRA_CONF_FILE=overlay-secure_storage.conf;overlay-transform_default.conf + - CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS=y + + secure_storage.psa.its.secure_storage.settings.nvs: + filter: CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_SETTINGS extra_args: "EXTRA_CONF_FILE=\ - overlay-secure_storage.conf;overlay-default_transform.conf;overlay-default_store.conf" - integration_platforms: - - native_sim - - nrf54l15dk/nrf54l15/cpuapp + overlay-secure_storage.conf;overlay-transform_default.conf;overlay-store_settings.conf" + secure_storage.psa.its.secure_storage.custom.transform: filter: CONFIG_SECURE_STORAGE and not CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_NONE - extra_args: "EXTRA_CONF_FILE=\ - overlay-secure_storage.conf;overlay-custom_transform.conf;overlay-default_store.conf" - integration_platforms: - - native_sim - - nrf54l15dk/nrf54l15/cpuapp + extra_args: EXTRA_CONF_FILE=overlay-secure_storage.conf;overlay-transform_custom.conf + secure_storage.psa.its.secure_storage.custom.store: filter: CONFIG_SECURE_STORAGE extra_args: "EXTRA_CONF_FILE=\ - overlay-secure_storage.conf;overlay-default_transform.conf;overlay-custom_store.conf" - integration_platforms: - - native_sim - - nrf54l15dk/nrf54l15/cpuapp + overlay-secure_storage.conf;overlay-transform_default.conf;overlay-store_custom.conf" + secure_storage.psa.its.secure_storage.custom.both: filter: CONFIG_SECURE_STORAGE extra_args: "EXTRA_CONF_FILE=\ - overlay-secure_storage.conf;overlay-custom_transform.conf;overlay-custom_store.conf" - integration_platforms: - - native_sim - - nrf54l15dk/nrf54l15/cpuapp + overlay-secure_storage.conf;overlay-transform_custom.conf;overlay-store_custom.conf" + secure_storage.psa.its.tfm: filter: CONFIG_BUILD_WITH_TFM - extra_args: EXTRA_CONF_FILE=overlay-tfm.conf integration_platforms: - nrf9151dk/nrf9151/ns + extra_args: EXTRA_CONF_FILE=overlay-tfm.conf diff --git a/tests/subsys/secure_storage/psa/its/zms.overlay b/tests/subsys/secure_storage/psa/its/zms.overlay new file mode 100644 index 0000000000000..6a739db514ce5 --- /dev/null +++ b/tests/subsys/secure_storage/psa/its/zms.overlay @@ -0,0 +1,5 @@ +/ { + chosen { + secure_storage_its_partition = &storage_partition; + }; +}; From 5f6043be15190ffc46057b4ce970d87ba906f0ae Mon Sep 17 00:00:00 2001 From: Tomi Fontanilles Date: Tue, 12 Nov 2024 13:39:16 +0200 Subject: [PATCH 3/7] secure_storage: its: store: settings: stop using SETTINGS_MAX_VAL_LEN Remove the hard restriction on CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE. SETTINGS_MAX_VAL_LEN is in practice not used by any settings backend implementation. Signed-off-by: Tomi Fontanilles --- subsys/secure_storage/src/its/store/settings.c | 1 - 1 file changed, 1 deletion(-) diff --git a/subsys/secure_storage/src/its/store/settings.c b/subsys/secure_storage/src/its/store/settings.c index d6942fbae6984..253e29afff250 100644 --- a/subsys/secure_storage/src/its/store/settings.c +++ b/subsys/secure_storage/src/its/store/settings.c @@ -11,7 +11,6 @@ #ifdef CONFIG_SECURE_STORAGE_ITS_IMPLEMENTATION_ZEPHYR #include -BUILD_ASSERT(SECURE_STORAGE_ITS_TRANSFORM_MAX_STORED_DATA_SIZE <= SETTINGS_MAX_VAL_LEN); #endif LOG_MODULE_DECLARE(secure_storage, CONFIG_SECURE_STORAGE_LOG_LEVEL); From 1177fc9bc55e1157f68d2ecd3c214b58f0d992bb Mon Sep 17 00:00:00 2001 From: Tomi Fontanilles Date: Thu, 14 Nov 2024 16:59:38 +0200 Subject: [PATCH 4/7] secure_storage: warn when there is no ITS store module implementation Output a CMake error when the ITS store module is enabled but no implementation ended up enabled (due to unfulfilled prerequisites). This is to make it more clear than undefined references at link time. Not a fatal error because CMake cannot fail for the twister filtering to work on the tests. Signed-off-by: Tomi Fontanilles --- subsys/secure_storage/src/its/CMakeLists.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/subsys/secure_storage/src/its/CMakeLists.txt b/subsys/secure_storage/src/its/CMakeLists.txt index 90340047ff5e3..ddb0a0265cef4 100644 --- a/subsys/secure_storage/src/its/CMakeLists.txt +++ b/subsys/secure_storage/src/its/CMakeLists.txt @@ -30,3 +30,9 @@ zephyr_library_sources_ifdef(CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS zephyr_library_sources_ifdef(CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_SETTINGS store/settings.c ) +if (CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_NONE) + message(ERROR " + The secure storage ITS module is enabled but has no implementation. + (CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_NONE) + ") +endif() From 748253df133fe6e7aff8f72f308337e7cd9b98f8 Mon Sep 17 00:00:00 2001 From: Tomi Fontanilles Date: Thu, 14 Nov 2024 17:00:28 +0200 Subject: [PATCH 5/7] secure_storage: its: store: settings: allow using zephyr,settings-partition Align with the behavior of the settings subsystem. Signed-off-by: Tomi Fontanilles --- subsys/secure_storage/Kconfig.its_store | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/subsys/secure_storage/Kconfig.its_store b/subsys/secure_storage/Kconfig.its_store index 99ac19e375737..05ebf72ca8f64 100644 --- a/subsys/secure_storage/Kconfig.its_store +++ b/subsys/secure_storage/Kconfig.its_store @@ -5,13 +5,15 @@ choice SECURE_STORAGE_ITS_STORE_IMPLEMENTATION prompt "ITS store module implementation" DT_ITS_PARTITION := $(dt_chosen_path,secure_storage_its_partition) +DT_CHOSEN_Z_SETTINGS_PARTITION := zephyr,settings-partition +DT_SETTINGS_PARTITIION := $(dt_chosen_path,$(DT_CHOSEN_Z_SETTINGS_PARTITION)) DT_STORAGE_PARTITION := $(dt_nodelabel_path,storage_partition) config SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS bool "ITS store module implementation using ZMS for storage" depends on FLASH_HAS_DRIVER_ENABLED \ - && $(dt_path_enabled,$(DT_ITS_PARTITION)) \ - && $(dt_node_has_compat,$(dt_node_parent,$(DT_ITS_PARTITION)),fixed-partitions) + && $(dt_path_enabled,$(DT_ITS_PARTITION)) \ + && $(dt_node_has_compat,$(dt_node_parent,$(DT_ITS_PARTITION)),fixed-partitions) select ZMS help This implementation of the ITS store module makes direct use of ZMS for storage. @@ -25,8 +27,10 @@ config SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS config SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_SETTINGS bool "ITS store module implementation using the settings subsystem for storage" depends on FLASH_HAS_DRIVER_ENABLED \ - && $(dt_path_enabled,$(DT_STORAGE_PARTITION)) \ - && $(dt_node_has_compat,$(dt_node_parent,$(DT_STORAGE_PARTITION)),fixed-partitions) + && (($(dt_path_enabled,$(DT_SETTINGS_PARTITIION)) \ + && $(dt_node_has_compat,$(dt_node_parent,$(DT_SETTINGS_PARTITIION)),fixed-partitions))\ + || ($(dt_path_enabled,$(DT_STORAGE_PARTITION)) \ + && $(dt_node_has_compat,$(dt_node_parent,$(DT_STORAGE_PARTITION)),fixed-partitions))) imply FLASH_MAP imply NVS select SETTINGS From 9cc91538c19aacaff1c9d92e23d17b027284436d Mon Sep 17 00:00:00 2001 From: Tomi Fontanilles Date: Thu, 14 Nov 2024 11:08:37 +0200 Subject: [PATCH 6/7] fs: zms: fix format string Use %zu for size_t. Signed-off-by: Tomi Fontanilles --- subsys/fs/zms/zms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subsys/fs/zms/zms.c b/subsys/fs/zms/zms.c index 57ac0ad52e01b..e00506dec4b4f 100644 --- a/subsys/fs/zms/zms.c +++ b/subsys/fs/zms/zms.c @@ -1362,7 +1362,7 @@ int zms_mount(struct zms_fs *fs) * 1 close ATE, 1 empty ATE, 1 GC done ATE, 1 Delete ATE, 1 ID/Value ATE */ if (fs->sector_size < ZMS_MIN_ATE_NUM * fs->ate_size) { - LOG_ERR("Invalid sector size, should be at least %u", + LOG_ERR("Invalid sector size, should be at least %zu", ZMS_MIN_ATE_NUM * fs->ate_size); } From 28c984ee994fd17b7541f1f69d3dc282d2c74c6b Mon Sep 17 00:00:00 2001 From: Tomi Fontanilles Date: Wed, 27 Nov 2024 12:20:04 +0200 Subject: [PATCH 7/7] secure_storage: its: store: settings: improve debug logging Align the debug logging with that of the ZMS-based implementation. Signed-off-by: Tomi Fontanilles --- subsys/secure_storage/src/its/store/settings.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/subsys/secure_storage/src/its/store/settings.c b/subsys/secure_storage/src/its/store/settings.c index 253e29afff250..e9725761201a1 100644 --- a/subsys/secure_storage/src/its/store/settings.c +++ b/subsys/secure_storage/src/its/store/settings.c @@ -20,7 +20,7 @@ static int init_settings_subsys(void) const int ret = settings_subsys_init(); if (ret) { - LOG_DBG("Failed to initialize the settings subsystem. (%d)", ret); + LOG_DBG("Failed. (%d)", ret); } return ret; } @@ -107,9 +107,7 @@ psa_status_t secure_storage_its_store_remove(secure_storage_its_uid_t uid) make_name(uid, name); ret = settings_delete(name); - if (ret) { - LOG_DBG("Failed to delete %s. (%d)", name, ret); - return PSA_ERROR_STORAGE_FAILURE; - } - return PSA_SUCCESS; + + LOG_DBG("%s %s. (%d)", ret ? "Failed to delete" : "Deleted", name, ret); + return ret ? PSA_ERROR_STORAGE_FAILURE : PSA_SUCCESS; }