core: tee_ree_fs: conditionally remove corrupt dirf.db file

[Ox-Yeh]

When a trusted application uses the REE-FS secure store API during the first boot, OP-TEE OS creates the dirf.db file. If a power outage occurs immediately after the creation of dirf.db, it may result in an empty file with an unset hash. This corrupted dirf.db file blocks subsequent REE-FS secure store operations. This commit addresses the issue by allowing the system to detect and delete an empty, corrupted dirf.db file, enabling secure store operations to proceed.

How to Simulate

1. Insert a delay (mdelay) immediately after the creation of dirf.db (see line 442 in tee_ree_fs.c), then power off the device.

422 static TEE_Result ree_fs_open_primitive(bool create, uint8_t *hash,
423 uint32_t min_counter,
424 const TEE_UUID *uuid,
425 struct tee_fs_dirfile_fileh *dfh,
426 struct tee_file_handle **fh)
427 {
428 TEE_Result res;
429 struct tee_fs_fd *fdp;
430
431 fdp = calloc(1, sizeof(struct tee_fs_fd));
432 if (!fdp)
433 return TEE_ERROR_OUT_OF_MEMORY;
434 fdp->fd = -1;
435 fdp->uuid = uuid;
436
437 if (create)
438 res = tee_fs_rpc_create_dfh(OPTEE_RPC_CMD_FS,
439 dfh, &fdp->fd);
440 else
441 res = tee_fs_rpc_open_dfh(OPTEE_RPC_CMD_FS, dfh, &fdp->fd);
442
443 if (res != TEE_SUCCESS)
444 goto out;

2. Upon reboot, check if the size of dirf.db is 0.

Effect of the Commit
After applying this commit, the system will automatically remove a corrupted dirf.db file for this power outage case (i.e., a file with size 0 with an unset hash), allowing subsequent secure store operations to function correctly.

[jforissier]

What if the power outage happens in the middle of writing data to dirf.db? That is, not immediately after the file is created (and still empty) but a little bit later so that data are only partially written. In this case, won't secure storage be marked as corrupt and become unusable?
Perhaps ree-fs_new_write() in tee-supplicant should use a special pattern to make sure writes are atomic (in the sense: either they occur in full or not at all). That can be accomplished by writing to a temporary copy, syncing then renaming the temporary file.

[Ox-Yeh]

Yes, a power outage during the writing process can result in a corrupted dirf.db file, making secure storage unusable.

Currently, the creation of dirf.db involves the following RPC steps:
1.Create the REE-FS DB file: dirf.db (initially 0 bytes)
2.Write 66 bytes at offset 4162 (node)
3.Write 68 bytes at offset 68 (head)
4.Write 68 bytes at offset 0 (head)
5.Close dirf.db

Although commit 5a9d570a attempts to handle power outages between steps 2 and 4 by checking ht_head_is_partially_done, it does not cover the case where a power outage occurs immediately after step 1.

Adding temporary copy in ree-fs_new_write() in tee-supplicant is a possible solution, but we still need to distinguish between corruption caused by power outages and potential REE-side attacks in TEE side. If the target device supports CFG_REE_FS_INTEGRITY_RPMB, maybe we shall directly refer to the status of dirfile.db.hash in RPMB for checking. For this case, we use file size:0 and no hash setting as the pattern from power outage.

[Ox-Yeh]

@jforissier
Are we good to move forward with this patch?

[jenswi-linaro]

I'll take another look at this.

[jenswi-linaro]

Why can hash be used to determine whether the file should be removed?
Wouldn't it make more sense to check if the file is too small (size 0?) and return TEE_ERROR_ITEM_NOT_FOUND?

[Ox-Yeh]

Hi Jenswi,

If an empty dirf.db file is present and no hash exists (possibly in RPMB), this situation typically results from an unexpected power outage during the initialization flow.

When the system attempts to read the dirf.db file, it calls init_head_from_data to reconstruct the hash tree. However, the RPC read command fails to retrieve the expected hash tree header—the returned buffer is zero and does not match the expected size of the hash tree header. As a result, the object is interpreted as corrupted, and the current implementation returns TEE_ERROR_CORRUPT_OBJECT.

Relevant code flow:

init_head_from_data
No hash case
rpc_read_head
rpc_read

Since rpc_read is a fundamental function and operates correctly, it is preferable to handle this error case in the control flow. Therefore, we recommend addressing this scenario in the ree_fs_open_primitive function, utilizing hash information to support proper error handling.

---

On the other hand, if there is an empty dirf.db file but a valid hash exists, it is more likely that the dirf.db file was modified on the REE side. In this case, we can still apply the same error handling flow to maintain consistency.

[jenswi-linaro]

If an empty dirf.db file is present and no hash exists (possibly in RPMB), this situation typically results from an unexpected power outage during the initialization flow.

What do you mean by "possibly in RPMB"? That it might be supplied with RPMB or not? Without RPMB, will it always be NULL or not?

[Ox-Yeh]

In my local environment, flag CFG_REE_FS_INTEGRITY_RPMB is enabled. However, without RPMB, it seems that no hash records are generated.

[jenswi-linaro]

So without CFG_REE_FS_INTEGRITY_RPMB enabled, you recommend to always remove "dirf.db" if it doesn't open correctly?

[Ox-Yeh]

Sorry for not mentioning this earlier—this change is based on the scenario where CFG_REE_FS_INTEGRITY_RPMB is enabled. If this flag is required, I can update the PR accordingly.

For cases without CFG_REE_FS_INTEGRITY_RPMB, it would be better to have an alternative method for handling hash records. Without hash records, I suggest keeping the current implementation or use flag: CFG_INSECURE to control whether automatically removing corrupted dirf.db if it doesn't open correctly.

[etienne-lms]

OP-TEE currently supports 2 ways to protect REE_FS against rollback: RPMB (based on a hash) and Monotonic Counter (nv_counter_get_ree_fs()). Should it be addressed here?
Suggestion:

		/*
		 * Recreate the database if the rollback protection does
		 * not protect its state.
		 */
		if (create ||
		    (IS_ENABLED(CFG_REE_FS_INTEGRITY_RPMB) && !hash) ||
		    (!IS_ENABLED(CFG_REE_FS_INTEGRITY_RPMB) && !min_counter)) {

which could factorize to if (create || (!hash && !min_counter)).

[Ox-Yeh]

Hi Etienne,
Thank you for providing clear instructions. I will update the pull request.