Persistent objects : save informations after close

[syedelec]

Hi,

I just wanted to notice that dataSize and dataPosition informations are not restored when opening an object after closing it. I thought it has been fixed in this pull request but I still have the problem, maybe I'm missing something but I don't understand what it is, here is the code I use :

Code :

TEE_ObjectHandle object = TEE_HANDLE_NULL;
TEE_ObjectInfo info;

int object_id = 0;

char write_data[50] = {0};
uint32_t write_data_len = 50;

uint32_t flags = TEE_DATA_FLAG_ACCESS_WRITE | TEE_DATA_FLAG_ACCESS_READ;

TEE_CreatePersistentObject(TEE_STORAGE_PRIVATE, &object_id, sizeof(object_id), 
                           flags, TEE_HANDLE_NULL, NULL, 0, &object);

TEE_WriteObjectData(object, write_data, write_data_len);

TEE_GetObjectInfo(object, &info);
MSG("Object data size : %zu and position : %zu", info.dataSize, info.dataPosition);

TEE_CloseObject(object);
TEE_OpenPersistentObject(TEE_STORAGE_PRIVATE, &object_id, sizeof(object_id), 
                         flags, &object);

TEE_GetObjectInfo(object, &info);
MSG("Object data size : %zu and position : %zu", info.dataSize, info.dataPosition);

Output :

Object data size : 50 and position : 50
Object data size : 0 and position : 0

I'm using QEMU and optee_os master.
Thank you for your help.

[jbech-linaro]

I've asked a colleague (Liang Guanchao) to look into this.

[hoihochan]

I see the same problem as well (on HiKey)

[hoihochan]

One thing I do notice if I supply the buffer pointer and size in TEE_CreatePersistentObject(...), the dataSize is correct.

I'm guessing when you call TEE_WriteObjectData(...), the meta header is not updated properly with the size where as TEE_CreatePersistentObject(...) does

[hoihochan]

I spent some time to debug this, and I'm pretty sure the root cause

TEE_Result syscall_storage_obj_write(...)
{
    ...

    if (o->info.dataPosition > o->info.dataSize) {
        o->info.dataSize = o->info.dataPosition;
    }

    ...
}

Although we update structure o, it does not persist back into storage, only the head structure does.

[hoihochan]

I do have a fix and would like to seek some advice before submitting a pull request.

• We store an extra pointer in struct tee_pobj for the file header
• On tee_svc_storage_read_head and tee_svc_storage_init_file, we do a malloc and store a copy of the file header
• On syscall_storage_obj_write, we write the updated file header back into storage if needed
• The file header in struct tee_pobj is properly freed when refcnt goes to 0.

On a second thought, the file header also contains other fields like objectUsage, which might change over time, maybe we need to take care of those as well?

Thanks
Donald

[jenswi-linaro]

Please submit a pull request, it's easier to discuss the fix when seeing the actual code.

[GuanchaoLiang]

Hi hoihochan,

I have also write a fix for the issue. What I do is as below:

• A static Func tee_svc_storage_update_head, what it do is to read the head from the FS, and then update the head.ds_size, finally rewrite the head.
• On syscall_storage_obj_write, it will invoke Func tee_svc_storage_update_head before it returns.

We may have a discussion here.

[hoihochan]

Pull request is here: #912

[GuanchaoLiang]

Hi,

I have review your pull request #912, it's much the same idea as mine.

[vchong]

@GuanchaoLiang If you've reviewed the pull request and are satisfied with the changes, please add your review tag to the PR. Something like this: Reviewed-by: Guanchao Liang <liang.guanchao@linaro.org>

[GuanchaoLiang]

@vchong Thanks, I will double check the code, and then add my review tag.

[jbech-linaro]

@hoihochan et al, seems like it's a bit more than a month ago since this Issue and #912 was updated. Is anyone actively working with this?

[hoihochan]

Sorry. I just came back from a month long vacation.

I will resume on it this week

On Monday, August 22, 2016, Joakim Bech notifications@github.com wrote:

@hoihochan https://github.com/hoihochan et al, seems like it's a bit
more than a month ago since this Issue and #912
#912 was updated. Is anyone
actively working with this?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#814 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAR8h4UjrejxHZKmnFaCdVoTbUqx4YpLks5qiWKNgaJpZM4InSaD
.

[jbech-linaro]

Great! I'm also just back after a month of vacation, so I've started to follow up a couple of things and that is why I asked :)

[jbech-linaro]

@hoihochan any updates? @GuanchaoLiang anything on your side?

[ghost]

Fixed in #1217, thanks for reporting the issue. I'm closing it now.