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

Modify obj usage for persistence object #6495

Closed
kyoWNC opened this issue Nov 25, 2023 · 5 comments · Fixed by #6505
Closed

Modify obj usage for persistence object #6495

kyoWNC opened this issue Nov 25, 2023 · 5 comments · Fixed by #6505
Labels

Comments

@kyoWNC
Copy link

kyoWNC commented Nov 25, 2023

optee-4.0.0
Case: Create a pure data object that can not share to CA

  1. Use the TEE_CreatePersistentObject API to create a pure data object (ID: obj1) (default obj usage is 0xffffffff)
  2. Modify the object usage to 0xfffffffe using TEE_RestrictObjectUsage1
  3. Call TEE_GetObjectInfo1 to check object usage, obtaining the result 0xfffffffe

But when using the TEE_GetObjectInfo1 API to get object usage from another CA, it will return 0xffffffff.
Is this the expected result?

Thanks

@jenswi-linaro
Copy link
Contributor

That's not expected. While looking at the code

TEE_Result syscall_cryp_obj_restrict_usage(unsigned long obj,
unsigned long usage)
{
struct ts_session *sess = ts_get_current_session();
TEE_Result res = TEE_SUCCESS;
struct tee_obj *o = NULL;
res = tee_obj_get(to_user_ta_ctx(sess->ctx), uref_to_vaddr(obj), &o);
if (res != TEE_SUCCESS)
goto exit;
o->info.objectUsage &= usage;
exit:
return res;
}

I can't find anything obvious. We don't have a test case for this in xtest. Could you create such a test case? With a test case to demonstrate the problem, it is much easier to fix it, besides a test case would guarantee that this remains working after an eventual fix has been merged.

@jenswi-linaro
Copy link
Contributor

I see how this might fail now. Yeah, this is a bug.

@kyoWNC
Copy link
Author

kyoWNC commented Nov 27, 2023

Hi jenswi-linaro,
I added the following code into regression_6000.c for the xtest.

static void xtest_tee_test_6021_single(ADBG_Case_t *c, uint32_t storage_id) 
{
	TEEC_Result res = TEEC_ERROR_GENERIC;
	TEEC_Session sess = { };
	uint32_t orig = 0;
	uint32_t obj = 0;
	TEE_ObjectInfo obj_info = { };
	
	res = xtest_teec_open_session(&sess, &storage_ta_uuid, NULL, &orig);
	if (!ADBG_EXPECT_TEEC_SUCCESS(c, res))
		return;

	res = fs_create(&sess, file_01, sizeof(file_01),
				TEE_DATA_FLAG_ACCESS_READ |
				TEE_DATA_FLAG_ACCESS_WRITE |
				TEE_DATA_FLAG_ACCESS_WRITE_META,
				0,
				data_00, sizeof(data_00),
				&obj,
				storage_id);
	if (!ADBG_EXPECT_TEEC_SUCCESS(c, res))
		goto exit;

	res = fs_get_obj_info(&sess, obj, &obj_info);
	if (!ADBG_EXPECT_TEEC_SUCCESS(c, res))
		goto exit;
	ADBG_EXPECT_COMPARE_UNSIGNED(c, obj_info.objectUsage, ==, 0xffffffff);

	res = fs_restrict_usage(&sess, obj, 0xfffffffe);
	if (!ADBG_EXPECT_TEEC_SUCCESS(c, res))
		goto exit;

	res = fs_get_obj_info(&sess, obj, &obj_info);
	if (!ADBG_EXPECT_TEEC_SUCCESS(c, res))
		goto exit;
	ADBG_EXPECT_COMPARE_UNSIGNED(c, obj_info.objectUsage, ==, 0xfffffffe);

	ADBG_EXPECT_TEEC_SUCCESS(c, fs_close(&sess, obj));
	TEEC_CloseSession(&sess);
	

	res = xtest_teec_open_session(&sess, &storage_ta_uuid, NULL, &orig);
	if (!ADBG_EXPECT_TEEC_SUCCESS(c, res))
		return;

	res = fs_open(&sess, file_01, sizeof(file_01),
				TEE_DATA_FLAG_ACCESS_READ |
				TEE_DATA_FLAG_ACCESS_WRITE |
				TEE_DATA_FLAG_ACCESS_WRITE_META, &obj, storage_id);
	if (!ADBG_EXPECT_TEEC_SUCCESS(c, res))
		goto exit;

	res = fs_get_obj_info(&sess, obj, &obj_info);
	if (!ADBG_EXPECT_TEEC_SUCCESS(c, res)) 
		goto exit;
	ADBG_EXPECT_COMPARE_UNSIGNED(c, obj_info.objectUsage, ==, 0xfffffffe);

exit:
	ADBG_EXPECT_TEEC_SUCCESS(c, fs_unlink(&sess, obj));
	TEEC_CloseSession(&sess);
	return;
}
DEFINE_TEST_MULTIPLE_STORAGE_IDS(xtest_tee_test_6021)
ADBG_CASE_DEFINE(regression, 6021, xtest_tee_test_6021,
		 "Modify and check object usage");

Thanks for your help

jenswi-linaro added a commit to jenswi-linaro/optee_os that referenced this issue Nov 29, 2023
Before this patch was the object usage field stored in the
non-persistent part of an object handle, regardless of whether the
storage object was persistent. This prevents updates to this field from
being restored the next time the persistent object is opened. Updates to
the field are also not replicated to eventual other open handles for the
object. Fix this by storing the "usage" bits in a new obj_info_usage
field in struct tee_pobj for persistent objects. Updates to the field
are also written into secure storage to preserve the content the next
time the object is opened.

Fixes: b010477 ("Open-source the TEE Core")
Closes: OP-TEE#6495
Signed-off-by: Jens Wiklander <[email protected]>
jenswi-linaro added a commit to jenswi-linaro/optee_test that referenced this issue Nov 29, 2023
Adds case 6021 to test modifying and checking object usage of a
persistent object.

Link: OP-TEE/optee_os#6495
Signed-off-by: Jens Wiklander <[email protected]>
@jenswi-linaro
Copy link
Contributor

Fixed in #6505

I can add a Reported-by: tag if you supply your name and email address.

The test is added in OP-TEE/optee_test#715

Feel free to provide your Signed-off-by: tag in the test PR if you want it included in the new test.

jenswi-linaro added a commit to jenswi-linaro/optee_os that referenced this issue Nov 29, 2023
Before this patch was the object usage field stored in the
non-persistent part of an object handle, regardless of whether the
storage object was persistent. This prevents updates to this field from
being restored the next time the persistent object is opened. Updates to
the field are also not replicated to eventual other open handles for the
object. Fix this by storing the "usage" bits in a new obj_info_usage
field in struct tee_pobj for persistent objects. Updates to the field
are also written into secure storage to preserve the content the next
time the object is opened.

Fixes: b010477 ("Open-source the TEE Core")
Closes: OP-TEE#6495
Signed-off-by: Jens Wiklander <[email protected]>
jenswi-linaro added a commit to jenswi-linaro/optee_os that referenced this issue Nov 29, 2023
Before this patch was the object usage field stored in the
non-persistent part of an object handle, regardless of whether the
storage object was persistent. This prevents updates to this field from
being restored the next time the persistent object is opened. Updates to
the field are also not replicated to eventual other open handles for the
object. Fix this by storing the "usage" bits in a new obj_info_usage
field in struct tee_pobj for persistent objects. Updates to the field
are also written into secure storage to preserve the content the next
time the object is opened.

Fixes: b010477 ("Open-source the TEE Core")
Closes: OP-TEE#6495
Signed-off-by: Jens Wiklander <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
jenswi-linaro added a commit to jenswi-linaro/optee_test that referenced this issue Nov 29, 2023
Adds case 6021 to test modifying and checking object usage of a
persistent object.

Link: OP-TEE/optee_os#6495
Signed-off-by: Jens Wiklander <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
jenswi-linaro added a commit to jenswi-linaro/optee_test that referenced this issue Nov 29, 2023
Add case 6021 to test modifying and checking object usage of a
persistent object.

Link: OP-TEE/optee_os#6495
Signed-off-by: Jens Wiklander <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
Reviewed-by: Etienne Carriere <[email protected]>
jenswi-linaro added a commit to jenswi-linaro/optee_os that referenced this issue Nov 29, 2023
Before this patch was the object usage field stored in the
non-persistent part of an object handle, regardless of whether the
storage object was persistent. This prevents updates to this field from
being restored the next time the persistent object is opened. Updates to
the field are also not replicated to eventual other open handles for the
object. Fix this by storing the "usage" bits in a new obj_info_usage
field in struct tee_pobj for persistent objects. Updates to the field
are also written into secure storage to preserve the content the next
time the object is opened.

Fixes: b010477 ("Open-source the TEE Core")
Closes: OP-TEE#6495
Signed-off-by: Jens Wiklander <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
Acked-by: Etienne Carriere <[email protected]>
@kyoWNC
Copy link
Author

kyoWNC commented Nov 29, 2023

It's ok.
The great thing is that the issue can be resolved :)
I will try this patch.
Thank you.

jforissier pushed a commit that referenced this issue Nov 30, 2023
Before this patch was the object usage field stored in the
non-persistent part of an object handle, regardless of whether the
storage object was persistent. This prevents updates to this field from
being restored the next time the persistent object is opened. Updates to
the field are also not replicated to eventual other open handles for the
object. Fix this by storing the "usage" bits in a new obj_info_usage
field in struct tee_pobj for persistent objects. Updates to the field
are also written into secure storage to preserve the content the next
time the object is opened.

Fixes: b010477 ("Open-source the TEE Core")
Closes: #6495
Signed-off-by: Jens Wiklander <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
Acked-by: Etienne Carriere <[email protected]>
jenswi-linaro added a commit to jenswi-linaro/optee_test that referenced this issue Nov 30, 2023
Add case 6021 to test modifying and checking object usage of a
persistent object.

Link: OP-TEE/optee_os#6495
Signed-off-by: Jens Wiklander <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
Reviewed-by: Etienne Carriere <[email protected]>
jforissier pushed a commit to OP-TEE/optee_test that referenced this issue Nov 30, 2023
Add case 6021 to test modifying and checking object usage of a
persistent object.

Link: OP-TEE/optee_os#6495
Signed-off-by: Jens Wiklander <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
Reviewed-by: Etienne Carriere <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants