Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected behavior may lead to dirf.db corruption #7037

Open
ZheTing815 opened this issue Sep 12, 2024 · 2 comments
Open

Unexpected behavior may lead to dirf.db corruption #7037

ZheTing815 opened this issue Sep 12, 2024 · 2 comments

Comments

@ZheTing815
Copy link
Contributor

When accessing dirf.db, if there is an unexpected operation during the process, such as a sudden power outage, it may lead to dirf.db corruption. The next time, OP-TEE try to open the corrupted dirf.db, it would receive the error code TEE_ERROR_CORRUPT_OBJECT. In such cases, can OP-TEE automatically reset dirf.db?
That is to say, can we also add a case for TEE_ERROR_CORRUPT_OBJECT at line 551, allowing it to be rebuilt just like TEE_ERROR_ITEM_NOT_FOUND?"

if (res == TEE_ERROR_ITEM_NOT_FOUND) {
if (hashp) {
if (IS_ENABLED(CFG_REE_FS_ALLOW_RESET)) {
DMSG("dirf.db not found, clear hash in RPMB");
res = rpmb_fs_ops.truncate(ree_fs_rpmb_fh, 0);
if (res) {
DMSG("Can't clear hash: %#"PRIx32, res);
res = TEE_ERROR_SECURITY;
goto out;
}
} else {
DMSG("dirf.db file not found");
res = TEE_ERROR_SECURITY;
goto out;
}
}
res = tee_fs_dirfile_open(true, NULL, 0, &ree_dirf_ops, dirh);
}

@etienne-lms
Copy link
Contributor

Allowing to blindly reset REE_FS secure storage could lead to some rollback protection issue IMHO. When using REE_FS secure storage, we expect the filesystem in the non-secure OS has some protection against power loss and like, like some journaling support.

@ZheTing815
Copy link
Contributor Author

Thank you for your explanation.
I conducted a tricky experiment. After creating the dirf.db in REE FS, I added some delay before storing to RPMB_FS then rebooting the device.
After that, opening the dirf.db will get the TEE_ERROR_CORRUPT_OBJECT.

diff --git a/core/tee/tee_ree_fs.c b/core/tee/tee_ree_fs.c
index 0b14640e2..ced7bd116 100644
--- a/core/tee/tee_ree_fs.c
+++ b/core/tee/tee_ree_fs.c
@@ -9,6 +9,7 @@
 #include <kernel/nv_counter.h>
 #include <kernel/panic.h>
 #include <kernel/thread.h>
+#include <kernel/delay.h>
 #include <kernel/user_access.h>
 #include <mempool.h>
 #include <mm/core_memprot.h>
@@ -440,6 +441,10 @@ static TEE_Result ree_fs_open_primitive(bool create, uint8_t *hash,
        else
                res = tee_fs_rpcc_open_dfh(OPTEE_RPC_CMD_FS, dfh, &fdp->fd);

+       if (create && !dfh) {
+               EMSG("After creating dirf.db in REE FS, delay and power off");
+               mdelay(10000);
+       }
        if (res != TEE_SUCCESS)
                goto out;

I think that this case cannot be covered by the filesystem in the non-secure OS.
Is my understanding wrong?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants