Skip to content

fix open issue 4577 TEE_SetTAPersistentTime() is not persistent accro… #7506

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix open issue 4577 TEE_SetTAPersistentTime() is not persistent accro… #7506

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
181 changes: 157 additions & 24 deletions core/tee/tee_time_generic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@
/*
* Copyright (c) 2014, STMicroelectronics International N.V.
*/
#define TA_TIME_OFFS_ID "ta_time_offs"

#include <kernel/mutex.h>
#include <kernel/panic.h>
#include <kernel/tee_time.h>
#include <string.h>
#include <stdlib.h>
#include <string.h>
#include <tee/tee_fs.h>
#include <tee/tee_pobj.h>
#include <utee_defines.h>

struct tee_ta_time_offs {
Expand All @@ -17,50 +21,179 @@ struct tee_ta_time_offs {

static struct tee_ta_time_offs *tee_time_offs;
static size_t tee_time_num_offs;
static struct mutex tee_time_mtx = MUTEX_INITIALIZER;

static TEE_Result tee_time_ta_get_offs(const TEE_UUID *uuid,
const TEE_Time **offs, bool *positive)
{
size_t n;

for (n = 0; n < tee_time_num_offs; n++) {
if (memcmp(uuid, &tee_time_offs[n].uuid, sizeof(TEE_UUID))
== 0) {
*offs = &tee_time_offs[n].offs;
*positive = tee_time_offs[n].positive;
return TEE_SUCCESS;
}
}
return TEE_ERROR_TIME_NOT_SET;
}

static TEE_Result tee_time_ta_set_offs(const TEE_UUID *uuid,
const TEE_Time *offs, bool positive)
static TEE_Result tee_time_ta_set_offs_mem(const TEE_UUID *uuid,
const TEE_Time *offs,
bool positive, size_t *out_idx)
{
size_t n;
struct tee_ta_time_offs *o;
size_t n = 0;
struct tee_ta_time_offs *o = NULL;

mutex_lock(&tee_time_mtx);
for (n = 0; n < tee_time_num_offs; n++) {
if (memcmp(uuid, &tee_time_offs[n].uuid, sizeof(TEE_UUID))
== 0) {
if (memcmp(uuid, &tee_time_offs[n].uuid,
sizeof(TEE_UUID)) == 0) {
tee_time_offs[n].offs = *offs;
tee_time_offs[n].positive = positive;
if (out_idx)
*out_idx = n;
mutex_unlock(&tee_time_mtx);
return TEE_SUCCESS;
}
}

n = tee_time_num_offs + 1;
o = realloc(tee_time_offs, n * sizeof(struct tee_ta_time_offs));
if (!o)
if (!o) {
mutex_unlock(&tee_time_mtx);
return TEE_ERROR_OUT_OF_MEMORY;
}
tee_time_offs = o;
tee_time_offs[tee_time_num_offs].uuid = *uuid;
tee_time_offs[tee_time_num_offs].offs = *offs;
tee_time_offs[tee_time_num_offs].positive = positive;
tee_time_num_offs = n;
if (out_idx)
*out_idx = n - 1;
mutex_unlock(&tee_time_mtx);
return TEE_SUCCESS;
}

static TEE_Result tee_time_ta_storage_write(const TEE_UUID *uuid,
const TEE_Time *offs, bool positive)
{
const struct tee_file_operations *fops =
tee_svc_storage_file_ops(TEE_STORAGE_PRIVATE);
struct tee_file_handle *fh = NULL;
struct tee_pobj *po = NULL;
struct tee_ta_time_offs o = { };
TEE_Result res = TEE_SUCCESS;

if (!fops)
return TEE_ERROR_NOT_SUPPORTED;

res = tee_pobj_get((TEE_UUID *)uuid, (void *)TA_TIME_OFFS_ID,
Copy link
Contributor

@jenswi-linaro jenswi-linaro Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The object will be created in the object namespace for the TA/UUID in question. What if a TA has an object with this name?

I think we can manage with a dummy pobj like in check_update_version() instead. This will allow us to create an object outside the normal namespace. However, we must still be careful to avoid conflicts.

sizeof(TA_TIME_OFFS_ID) - 1,
TEE_DATA_FLAG_ACCESS_WRITE,
TEE_POBJ_USAGE_CREATE, fops, &po);

if (res)
return res;

o.uuid = *uuid;
o.offs = *offs;
o.positive = positive;

res = fops->create(po, true, NULL, 0, NULL, 0, &o, NULL, sizeof(o),
&fh);
Copy link
Contributor

@jenswi-linaro jenswi-linaro Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please align with the opening (.


if (!res) {
fops->close(&fh);
tee_pobj_create_final(po);
}
tee_pobj_release(po);
return res;
}

static TEE_Result tee_time_ta_storage_read(const TEE_UUID *uuid,
TEE_Time *offs, bool *positive)
{
const struct tee_file_operations *fops =
Copy link
Contributor

@jenswi-linaro jenswi-linaro Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please initialize with NULL here and assign it below for cleaner code flow.

tee_svc_storage_file_ops(TEE_STORAGE_PRIVATE);
struct tee_file_handle *fh = NULL;
struct tee_pobj *po = NULL;
TEE_Result res = TEE_SUCCESS;
size_t sz = 0;
struct tee_ta_time_offs o = { };

if (!fops)
return TEE_ERROR_TIME_NOT_SET;

res = tee_pobj_get((TEE_UUID *)uuid, (void *)TA_TIME_OFFS_ID,
sizeof(TA_TIME_OFFS_ID) - 1,
TEE_DATA_FLAG_ACCESS_READ, TEE_POBJ_USAGE_OPEN,
fops, &po);

if (res) {
if (res == TEE_ERROR_ITEM_NOT_FOUND)
return TEE_ERROR_TIME_NOT_SET;
return res;
}

res = fops->open(po, &sz, &fh);
if (!res) {
sz = sizeof(o);
res = fops->read(fh, 0, &o, NULL, &sz);
fops->close(&fh);
}

tee_pobj_release(po);

if (res) {
if (res == TEE_ERROR_ITEM_NOT_FOUND)
return TEE_ERROR_TIME_NOT_SET;
return res;
}

if (sz != sizeof(o))
return TEE_ERROR_TIME_NOT_SET;

*offs = o.offs;
*positive = o.positive;
return TEE_SUCCESS;
}

static TEE_Result tee_time_ta_get_offs(const TEE_UUID *uuid,
const TEE_Time **offs, bool *positive)
{
size_t n = 0;
TEE_Time o;
bool pos = false;
size_t idx = 0;
TEE_Result res = TEE_ERROR_GENERIC;

mutex_lock(&tee_time_mtx);
for (n = 0; n < tee_time_num_offs; n++) {
if (memcmp(uuid, &tee_time_offs[n].uuid, sizeof(TEE_UUID))
Copy link
Contributor

@jenswi-linaro jenswi-linaro Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer if (!memcmp...).

== 0) {
*offs = &tee_time_offs[n].offs;
*positive = tee_time_offs[n].positive;
mutex_unlock(&tee_time_mtx);
return TEE_SUCCESS;
}
}

mutex_unlock(&tee_time_mtx);
res = tee_time_ta_storage_read(uuid, &o, &pos);

if (res)
return res;

res = tee_time_ta_set_offs_mem(uuid, &o, pos, &idx);
if (res)
return res;

mutex_lock(&tee_time_mtx);
*offs = &tee_time_offs[idx].offs;
*positive = tee_time_offs[idx].positive;
mutex_unlock(&tee_time_mtx);

return TEE_SUCCESS;
}

static TEE_Result tee_time_ta_set_offs(const TEE_UUID *uuid,
const TEE_Time *offs, bool positive)
{
size_t idx = 0;
TEE_Result res = TEE_ERROR_GENERIC;

res = tee_time_ta_set_offs_mem(uuid, offs, positive, &idx);
if (res)
return res;
return tee_time_ta_storage_write(uuid, offs, positive);
Copy link
Contributor

@jenswi-linaro jenswi-linaro Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if tee_time_ta_storage_write() fails? Then we have an inconsistent state.

}

TEE_Result tee_time_get_ta_time(const TEE_UUID *uuid, TEE_Time *time)
{
TEE_Result res;
Expand Down
Loading