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

Restric usage #715

Merged
merged 2 commits into from
Nov 30, 2023
Merged

Restric usage #715

merged 2 commits into from
Nov 30, 2023

Conversation

jenswi-linaro
Copy link
Contributor

Depends on the fix in OP-TEE/optee_os#6505

@jenswi-linaro
Copy link
Contributor Author

The failure is expected since there's an added test for something that is fixed in OP-TEE/optee_os#6505.

@jforissier
Copy link
Contributor

For "ta: storage: fix ta_storage_cmd_key_in_persistent() cleanup":

Reviewed-by: Jerome Forissier <[email protected]>

@jforissier
Copy link
Contributor

For "xtest: regression: add case 6021 object usage":

Reviewed-by: Jerome Forissier <[email protected]>

@jenswi-linaro
Copy link
Contributor Author

Tags applied.

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Etienne Carriere <[email protected]> for commit "TEE_DATA_FLAG_OVERWRITE"
"ta: storage: fix ta_storage_cmd_key_in_persistent() cleanup"
with imperative mode in commit message.

edited: fix commit reference that was obviously not accurate

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);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest to clear an explicitly meaningful usage bitflag:

	ADBG_EXPECT_COMPARE_UNSIGNED(c, obj_info.objectUsage, ==,
				     0xffffffff & ~TEE_USAGE_EXTRACTABLE);

}
DEFINE_TEST_MULTIPLE_STORAGE_IDS(xtest_tee_test_6021)
ADBG_CASE_DEFINE(regression, 6021, xtest_tee_test_6021,
"Modify and check object usage");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: "Modify and check persistent object usage"

{
const uint32_t flags = TEE_DATA_FLAG_ACCESS_READ |
TEE_DATA_FLAG_ACCESS_WRITE |
TEE_DATA_FLAG_ACCESS_WRITE_META;
Copy link
Contributor

Choose a reason for hiding this comment

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

TEE_DATA_FLAG_OVERWRITE to ensure object is destroyed and recreated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll add it to the create call.

@jenswi-linaro
Copy link
Contributor Author

Addressing comments, tag applied.

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, ==, ou);
Copy link
Contributor

Choose a reason for hiding this comment

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

goto exit here on failure I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test has failed at this point, but it's not completely useless to open a new session, reopen the object, and check once more. The test is stable even on errors.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok

@etienne-lms
Copy link
Contributor

Sorry for the bad reference for my R-b tag.
Whatever, Reviewed-by: Etienne Carriere <[email protected]> for both commits.

Fixes the error cleanup path in two places where a created test object
was supposed to be removed.

Fixes: 90f2335 ("Test Persistent Object with keys")
Signed-off-by: Jens Wiklander <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
Reviewed-by: Etienne Carriere <[email protected]>
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
Copy link
Contributor Author

Tag applied

@jforissier jforissier merged commit 1033462 into OP-TEE:master Nov 30, 2023
2 checks passed
@jenswi-linaro jenswi-linaro deleted the restric_usage branch November 30, 2023 14:04
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

Successfully merging this pull request may close these issues.

3 participants