tests: subsys: secure_storage: Erase flash before write once test #99701

petejohanson-adi · 2025-11-19T21:37:46Z

Where possible (i.e. not running in NS), erase the flash before testing the write once secure storage API.

This is based on feedback here: #97529 (review)

Note: This does not help with NS targets that can't directly access the flash and erase it. @tomi-font I'd really appreciate if you could offer some ideas for how to address that problem other than the one proposed in #97529

Where possible (i.e. not running in NS), erase the flash before testing the write once secure storage API. Signed-off-by: Pete Johanson <[email protected]>

dsseng · 2025-11-19T21:45:16Z

Are NS targets even supposed to use secure_storage? I believe if you cannot access the flash from NS, then you would typically use PSA ITS/PS provided by the secure world, thus not needing Zephyr's implementation

petejohanson-adi · 2025-11-19T21:52:12Z

In our particular case, we have a possible convoluted path of secure_storage -> settings -> settings ITS backend.

sonarqubecloud · 2025-11-19T21:52:33Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

dsseng · 2025-11-19T21:51:22Z

tests/subsys/secure_storage/psa/its/src/main.c

+#if defined(CONFIG_FLASH_HAS_NO_EXPLICIT_ERASE)
+	const struct flash_parameters *fparam = flash_get_parameters(fdev);
+
+	if (!(flash_params_get_erase_cap(fparam) & FLASH_ERASE_C_EXPLICIT)) {


Maybe in case we have no erase operation (e.g. nRF54 RRAM and MRAM) we should fully overwrite the region with zeroes or ones? That would also erase the data to make sure we start from a known-empty state.

native_sim can also simulate such NVM model IIRC, so it should be testable even without such HW.

dsseng · 2025-11-19T21:51:43Z

tests/subsys/secure_storage/psa/its/src/main.c

+
+	for (int i = 0; i < MAX_NUM_PAGES && address < FLASH_ERASE_END_ADDR; i++) {
+		rc = flash_get_page_info_by_offs(fdev, address, &page_info);
+


non-blocking nit to unify the code style here

Suggested change

dsseng · 2025-11-19T21:54:24Z

In our particular case, we have a possible convoluted path of secure_storage -> settings -> settings ITS backend.

Yes, nothing prevents this from working, but perhaps only running this test without TF-M is sufficient for the coverage purposes

tomi-font

@dsseng is right in that NS targets are not meant to use Secure Storage:

zephyr/subsys/secure_storage/Kconfig

Lines 4 to 6 in f1c2ce4

    
           menuconfig SECURE_STORAGE 
        
           	bool "Secure storage subsystem" 
        
           	depends on !BUILD_WITH_TFM

However this test has a scenario for TF-M in order to test API compatibility between TF-M and Secure Storage (as all the test does is use the PSA ITS API):

zephyr/tests/subsys/secure_storage/psa/its/testcase.yaml

Lines 67 to 71 in f1c2ce4

    
           secure_storage.psa.its.tfm: 
        
             filter: CONFIG_BUILD_WITH_TFM 
        
             integration_platforms: 
        
               - nrf9151dk/nrf9151/ns 
        
             extra_args: EXTRA_CONF_FILE=overlay-tfm.conf

So it makes sense to also consider TF-M here.

In our particular case, we have a possible convoluted path of secure_storage -> settings -> settings ITS backend.

This is really weird... Can you tell me more about your use case? Why wouldn't you just directly be using the PSA ITS API exposed by TF-M, and how can you even manage to do this as there must be symbol clashes?

Other than that, thanks for the PR, I tested and it works on the nRF9151 and on native_sim with the Settings backend. Although not:

On the nRF54L15, probably because of https://github.com/zephyrproject-rtos/zephyr/pull/99701/files#r2543678217; could you also make it work there?

With the ZMS backend. In that case (on native_sim) the return value of psa_its_get_info() is not PSA_SUCCESS, which makes the test fail. This is maybe because the erase operation messes up ZMS or that the mounting needs to be done after erasing the flash:

zephyr/subsys/secure_storage/src/its/store/zms.c

Lines 22 to 34 in bd9dc90

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

Maybe having the flash erase as a SYS_INIT() hook that would run before that of ZMS would work.

Note: This does not help with NS targets that can't directly access the flash and erase it. @tomi-font I'd really appreciate if you could offer some ideas for how to address that problem other than the one proposed in #97529

I'm afraid there's no way around that, for NS targets you will just need to run twister/west with the erase option. It would be a pretty big security hole if the NS application could just erase parts of the secure flash. I mean I imagine it must be possible some way to achieve that but it must be all but trivial.

tomi-font · 2025-11-20T09:09:31Z

tests/subsys/secure_storage/psa/its/src/main.c

+
+#if !defined(CONFIG_BUILD_WITH_TFM) && defined(CONFIG_FLASH_HAS_EXPLICIT_ERASE)
+
+#define MAX_NUM_PAGES 2


How did you come up with this value? Is it even needed?

tomi-font · 2025-11-20T10:10:20Z

tests/subsys/secure_storage/psa/its/src/main.c

+	}
+}
+
+#endif


Suggested change

#endif

#endif /* !CONFIG_BUILD_WITH_TFM && CONFIG_FLASH_HAS_EXPLICIT_ERASE */

tomi-font · 2025-11-20T10:10:40Z

tests/subsys/secure_storage/psa/its/src/main.c

+	if (!(flash_params_get_erase_cap(fparam) & FLASH_ERASE_C_EXPLICIT)) {
+		return;
+	}
+#endif


Suggested change

#endif

#endif /* CONFIG_FLASH_HAS_NO_EXPLICIT_ERASE */

dsseng · 2025-11-20T11:25:57Z

I'm afraid there's no way around that, for NS targets you will just need to run twister/west with the erase option. It would be a pretty big security hole if the NS application could just erase parts of the secure flash. I mean I imagine it must be possible some way to achieve that but it must be all but trivial.

Can be potentially hacked around by making TF-M skip setting security attributes to the flash controller (needs a test-specific patch). It's still not really worth it for the test though it seems.

And on properly secured hardware that's the goal of TF-M to prevent NSPE from accessing Secure hardware, so there's no universal solution without hacking on TF-M.

@petejohanson-adi I still fail to grasp what do you test in such a case? If you use Secure Storage, then it writes to somewhere accessible from NSPE, right? Then that region should also be erasable/rewriteable by NSPE.

If you meant Secure Storage on top of ITS provided by TF-M (yes, I now realized it shouldn't even compile due to TF-M runtime library providing the same symbol names as secure_storage), please take a look here, maybe you should wipe ITS entries, and not the backing Flash: https://github.com/zephyrproject-rtos/zephyr/pull/94881/files#diff-93455f3e147570503f332b1446633676ffb36a58f958179f334273ca6b30c78e

tests: subsys: secure_storage: Erase flash before write once test

ec97597

Where possible (i.e. not running in NS), erase the flash before testing the write once secure storage API. Signed-off-by: Pete Johanson <[email protected]>

zephyrbot added area: Secure Storage Secure Storage area: Tests Issues related to a particular existing or missing test labels Nov 19, 2025

zephyrbot requested review from nashif and tomi-font November 19, 2025 21:39

zephyrbot assigned tomi-font Nov 19, 2025

dsseng reviewed Nov 19, 2025

View reviewed changes

tomi-font reviewed Nov 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

tests: subsys: secure_storage: Erase flash before write once test #99701

tests: subsys: secure_storage: Erase flash before write once test #99701

Uh oh!

petejohanson-adi commented Nov 19, 2025

Uh oh!

dsseng commented Nov 19, 2025

Uh oh!

petejohanson-adi commented Nov 19, 2025

Uh oh!

sonarqubecloud bot commented Nov 19, 2025

Uh oh!

dsseng Nov 19, 2025

Uh oh!

dsseng Nov 19, 2025

Uh oh!

dsseng commented Nov 19, 2025

Uh oh!

tomi-font left a comment

Uh oh!

tomi-font Nov 20, 2025

Uh oh!

tomi-font Nov 20, 2025

Uh oh!

tomi-font Nov 20, 2025

Uh oh!

dsseng commented Nov 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants


		for (int i = 0; i < MAX_NUM_PAGES && address < FLASH_ERASE_END_ADDR; i++) {
		rc = flash_get_page_info_by_offs(fdev, address, &page_info);

	menuconfig SECURE_STORAGE
	bool "Secure storage subsystem"
	depends on !BUILD_WITH_TFM

	secure_storage.psa.its.tfm:
	filter: CONFIG_BUILD_WITH_TFM
	integration_platforms:
	- nrf9151dk/nrf9151/ns
	extra_args: EXTRA_CONF_FILE=overlay-tfm.conf

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


		#if !defined(CONFIG_BUILD_WITH_TFM) && defined(CONFIG_FLASH_HAS_EXPLICIT_ERASE)

		#define MAX_NUM_PAGES 2

+              	}
+              }
+              #endif

	#endif
	#endif /* !CONFIG_BUILD_WITH_TFM && CONFIG_FLASH_HAS_EXPLICIT_ERASE */

tests: subsys: secure_storage: Erase flash before write once test #99701

Are you sure you want to change the base?

tests: subsys: secure_storage: Erase flash before write once test #99701

Uh oh!

Conversation

petejohanson-adi commented Nov 19, 2025

Uh oh!

dsseng commented Nov 19, 2025

Uh oh!

petejohanson-adi commented Nov 19, 2025

Uh oh!

sonarqubecloud bot commented Nov 19, 2025

Quality Gate passed

Uh oh!

dsseng Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

dsseng Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

dsseng commented Nov 19, 2025

Uh oh!

tomi-font left a comment

Choose a reason for hiding this comment

Uh oh!

tomi-font Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

tomi-font Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

tomi-font Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

dsseng commented Nov 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants