xtest storage: check that we can't use object_id from shared memory
According to the GP spec V1.1, the object_id in create/open/rename
functions is not allowed to reside in the shared memory.
This change checks that when a TA uses a SHM buffer to carry the
object_id parameter for create/open/rename function do rejects the
request. It is expected that the TA dies on such conditions.
Suggested-by: Zeng Tao <prime.zeng@hisilicon.com>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
diff --git a/host/xtest/regression_6000.c b/host/xtest/regression_6000.c
index 336ea14..c0e8e90 100644
--- a/host/xtest/regression_6000.c
+++ b/host/xtest/regression_6000.c
@@ -1971,6 +1971,223 @@
DEFINE_TEST_MULTIPLE_STORAGE_IDS(xtest_tee_test_6019)
+/*
+ * According to the GP spec V1.1, the object_id in create/open/rename
+ * functions is not allowed to reside in the shared memory.
+ *
+ * The function below replicates fs_open/fs_create/fs_rename but using
+ * specific commands to ask the TA to use the client object ID buffer
+ * from the shared memory when accessing the object through target APIs.
+ * The TA is not expected to use such references and gets killed by the TEE.
+ */
+static TEEC_Result fs_access_with_bad_object_id_ref(TEEC_Session *sess,
+ uint32_t command,
+ void *id, uint32_t id_size,
+ uint32_t flags,
+ uint32_t attr,
+ void *data, uint32_t data_size,
+ uint32_t *obj,
+ uint32_t storage_id)
+{
+ TEEC_Operation op = TEEC_OPERATION_INITIALIZER;
+ TEEC_Result res;
+ uint32_t org;
+
+ switch (command) {
+ case TA_STORAGE_CMD_OPEN_ID_IN_SHM:
+ op.params[0].tmpref.buffer = id;
+ op.params[0].tmpref.size = id_size;
+ op.params[1].value.a = flags;
+ op.params[1].value.b = 0;
+ op.params[2].value.a = storage_id;
+
+ op.paramTypes = TEEC_PARAM_TYPES(TEEC_MEMREF_TEMP_INPUT,
+ TEEC_VALUE_INOUT,
+ TEEC_VALUE_INPUT,
+ TEEC_NONE);
+ break;
+ case TA_STORAGE_CMD_CREATE_ID_IN_SHM:
+ op.params[0].tmpref.buffer = id;
+ op.params[0].tmpref.size = id_size;
+ op.params[1].value.a = flags;
+ op.params[1].value.b = 0;
+ op.params[2].value.a = attr;
+ op.params[2].value.b = storage_id;
+ op.params[3].tmpref.buffer = data;
+ op.params[3].tmpref.size = data_size;
+
+ op.paramTypes = TEEC_PARAM_TYPES(TEEC_MEMREF_TEMP_INPUT,
+ TEEC_VALUE_INOUT,
+ TEEC_VALUE_INPUT,
+ TEEC_MEMREF_TEMP_INPUT);
+ break;
+ case TA_STORAGE_CMD_CREATEOVER_ID_IN_SHM:
+ op.params[0].tmpref.buffer = id;
+ op.params[0].tmpref.size = id_size;
+ op.params[1].value.a = storage_id;
+
+ op.paramTypes = TEEC_PARAM_TYPES(TEEC_MEMREF_TEMP_INPUT,
+ TEEC_VALUE_INPUT,
+ TEEC_NONE, TEEC_NONE);
+ break;
+ case TA_STORAGE_CMD_RENAME_ID_IN_SHM:
+ op.params[0].value.a = *obj;
+ op.params[1].tmpref.buffer = id;
+ op.params[1].tmpref.size = id_size;
+
+ op.paramTypes = TEEC_PARAM_TYPES(TEEC_VALUE_INPUT,
+ TEEC_MEMREF_TEMP_INPUT,
+ TEEC_NONE, TEEC_NONE);
+ break;
+ default:
+ return TEE_ERROR_GENERIC;
+ }
+
+ res = TEEC_InvokeCommand(sess, command, &op, &org);
+
+ switch (command) {
+ case TA_STORAGE_CMD_OPEN_ID_IN_SHM:
+ case TA_STORAGE_CMD_CREATE_ID_IN_SHM:
+ if (res == TEEC_SUCCESS)
+ *obj = op.params[1].value.b;
+ break;
+ default:
+ break;
+ }
+
+ return res;
+}
+
+static void xtest_tee_test_6020_single(ADBG_Case_t *c, uint32_t storage_id)
+{
+ TEEC_Result res;
+ TEEC_Session sess;
+ uint32_t orig;
+ uint32_t obj;
+
+ /*
+ * Invalid open request from the TA (object ID reference in SHM)
+ */
+ 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_WRITE |
+ TEE_DATA_FLAG_ACCESS_WRITE_META |
+ TEE_DATA_FLAG_OVERWRITE,
+ 0,
+ data_00, sizeof(data_00),
+ &obj,
+ storage_id);
+
+ if (!ADBG_EXPECT_TEEC_SUCCESS(c, res))
+ goto exit1;
+ if (!ADBG_EXPECT_TEEC_SUCCESS(c, fs_close(&sess, obj)))
+ goto exit1;
+
+ res = fs_access_with_bad_object_id_ref(&sess,
+ TA_STORAGE_CMD_OPEN_ID_IN_SHM,
+ file_01, sizeof(file_01),
+ TEE_DATA_FLAG_ACCESS_WRITE |
+ TEE_DATA_FLAG_ACCESS_WRITE_META |
+ TEE_DATA_FLAG_OVERWRITE,
+ 0,
+ NULL, 0,
+ &obj,
+ storage_id);
+
+ ADBG_EXPECT_TEEC_RESULT(c, TEE_ERROR_TARGET_DEAD, res);
+
+ /*
+ * Invalid create-overwrite request from the TA (object ID reference in SHM)
+ */
+ TEEC_CloseSession(&sess);
+ res = xtest_teec_open_session(&sess, &storage_ta_uuid, NULL, &orig);
+ if (!ADBG_EXPECT_TEEC_SUCCESS(c, res))
+ return;
+
+ res = fs_access_with_bad_object_id_ref(&sess,
+ TA_STORAGE_CMD_CREATEOVER_ID_IN_SHM,
+ file_01, sizeof(file_01),
+ 0,
+ 0,
+ NULL, 0,
+ NULL,
+ storage_id);
+
+ ADBG_EXPECT_TEEC_RESULT(c, TEE_ERROR_TARGET_DEAD, res);
+
+ /*
+ * Invalid rename request from the TA (object ID reference in SHM)
+ */
+ 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_WRITE |
+ TEE_DATA_FLAG_ACCESS_WRITE_META |
+ TEE_DATA_FLAG_OVERWRITE,
+ &obj,
+ storage_id);
+
+ if (!ADBG_EXPECT_TEEC_SUCCESS(c, res))
+ goto exit1;
+
+ res = fs_access_with_bad_object_id_ref(&sess,
+ TA_STORAGE_CMD_RENAME_ID_IN_SHM,
+ file_01, sizeof(file_01) - 1,
+ 0,
+ 0,
+ NULL, 0,
+ &obj,
+ 0);
+
+ ADBG_EXPECT_TEEC_RESULT(c, TEE_ERROR_TARGET_DEAD, res);
+
+ /*
+ * Invalid creation request from the TA (object ID reference in SHM)
+ */
+ 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_WRITE |
+ TEE_DATA_FLAG_ACCESS_WRITE_META |
+ TEE_DATA_FLAG_OVERWRITE,
+ &obj,
+ storage_id);
+
+ if (!ADBG_EXPECT_TEEC_SUCCESS(c, res))
+ goto exit1;
+
+ ADBG_EXPECT_TEEC_SUCCESS(c, fs_unlink(&sess, obj));
+
+ res = fs_access_with_bad_object_id_ref(&sess,
+ TA_STORAGE_CMD_CREATE_ID_IN_SHM,
+ file_01, sizeof(file_01),
+ TEE_DATA_FLAG_ACCESS_WRITE |
+ TEE_DATA_FLAG_ACCESS_WRITE_META |
+ TEE_DATA_FLAG_OVERWRITE,
+ 0,
+ data_00, sizeof(data_00),
+ &obj,
+ storage_id);
+
+ ADBG_EXPECT_TEEC_RESULT(c, TEE_ERROR_TARGET_DEAD, res);
+
+ return;
+exit1:
+ ADBG_EXPECT_TEEC_SUCCESS(c, fs_unlink(&sess, obj));
+ TEEC_CloseSession(&sess);
+}
+
+DEFINE_TEST_MULTIPLE_STORAGE_IDS(xtest_tee_test_6020)
+
ADBG_CASE_DEFINE(regression, 6001, xtest_tee_test_6001,
"Test TEE_CreatePersistentObject");
ADBG_CASE_DEFINE(regression, 6002, xtest_tee_test_6002,
@@ -2008,3 +2225,5 @@
"Test Persistent objects info");
ADBG_CASE_DEFINE(regression, 6018, xtest_tee_test_6018, "Large object");
ADBG_CASE_DEFINE(regression, 6019, xtest_tee_test_6019, "Storage independence");
+ADBG_CASE_DEFINE(regression, 6020, xtest_tee_test_6020,
+ "Object IDs in SHM (negative)");
diff --git a/ta/include/ta_storage.h b/ta/include/ta_storage.h
index c5e8d38..f0f892c 100644
--- a/ta/include/ta_storage.h
+++ b/ta/include/ta_storage.h
@@ -55,5 +55,9 @@
#define TA_STORAGE_CMD_FREE_OBJ 19
#define TA_STORAGE_CMD_RESET_OBJ 20
#define TA_STORAGE_CMD_GET_OBJ_INFO 21
+#define TA_STORAGE_CMD_OPEN_ID_IN_SHM 22
+#define TA_STORAGE_CMD_CREATE_ID_IN_SHM 23
+#define TA_STORAGE_CMD_CREATEOVER_ID_IN_SHM 24
+#define TA_STORAGE_CMD_RENAME_ID_IN_SHM 25
#endif /*__TA_STORAGE_H*/
diff --git a/ta/storage/include/storage.h b/ta/storage/include/storage.h
index d4a4ddc..b0a1575 100644
--- a/ta/storage/include/storage.h
+++ b/ta/storage/include/storage.h
@@ -30,16 +30,20 @@
#include <tee_api.h>
-TEE_Result ta_storage_cmd_open(uint32_t param_types, TEE_Param params[4]);
-TEE_Result ta_storage_cmd_create(uint32_t param_types, TEE_Param params[4]);
-TEE_Result ta_storage_cmd_create_overwrite(uint32_t param_types,
+TEE_Result ta_storage_cmd_open(uint32_t command, uint32_t param_types,
+ TEE_Param params[4]);
+TEE_Result ta_storage_cmd_create(uint32_t command, uint32_t param_types,
+ TEE_Param params[4]);
+TEE_Result ta_storage_cmd_create_overwrite(uint32_t command,
+ uint32_t param_types,
TEE_Param params[4]);
TEE_Result ta_storage_cmd_close(uint32_t param_types, TEE_Param params[4]);
TEE_Result ta_storage_cmd_read(uint32_t param_types, TEE_Param params[4]);
TEE_Result ta_storage_cmd_write(uint32_t param_types, TEE_Param params[4]);
TEE_Result ta_storage_cmd_seek(uint32_t param_types, TEE_Param params[4]);
TEE_Result ta_storage_cmd_unlink(uint32_t param_types, TEE_Param params[4]);
-TEE_Result ta_storage_cmd_rename(uint32_t param_types, TEE_Param params[4]);
+TEE_Result ta_storage_cmd_rename(uint32_t command, uint32_t param_types,
+ TEE_Param params[4]);
TEE_Result ta_storage_cmd_trunc(uint32_t param_types, TEE_Param params[4]);
TEE_Result ta_storage_cmd_alloc_enum(uint32_t param_types, TEE_Param params[4]);
TEE_Result ta_storage_cmd_free_enum(uint32_t param_types, TEE_Param params[4]);
diff --git a/ta/storage/storage.c b/ta/storage/storage.c
index 6d97597..c61d548 100644
--- a/ta/storage/storage.c
+++ b/ta/storage/storage.c
@@ -26,6 +26,7 @@
*/
#include "storage.h"
+#include "ta_storage.h"
#include <tee_api.h>
#include <trace.h>
@@ -38,12 +39,12 @@
#define VAL2HANDLE(v) (void *)(uintptr_t)(v)
-TEE_Result ta_storage_cmd_open(uint32_t param_types, TEE_Param params[4])
+TEE_Result ta_storage_cmd_open(uint32_t command,
+ uint32_t param_types, TEE_Param params[4])
{
TEE_Result res;
TEE_ObjectHandle o;
void *object_id;
- size_t object_id_size;
ASSERT_PARAM_TYPE(TEE_PARAM_TYPES
(TEE_PARAM_TYPE_MEMREF_INPUT,
@@ -51,30 +52,41 @@
TEE_PARAM_TYPE_VALUE_INPUT,
TEE_PARAM_TYPE_NONE));
- object_id_size = params[0].memref.size;
- object_id = TEE_Malloc(object_id_size, 0);
- if (!object_id)
- return TEE_ERROR_OUT_OF_MEMORY;
+ switch (command) {
+ case TA_STORAGE_CMD_OPEN:
+ object_id = TEE_Malloc(params[0].memref.size, 0);
+ if (!object_id)
+ return TEE_ERROR_OUT_OF_MEMORY;
- TEE_MemMove(object_id, params[0].memref.buffer, object_id_size);
+ TEE_MemMove(object_id, params[0].memref.buffer,
+ params[0].memref.size);
+ break;
+ case TA_STORAGE_CMD_OPEN_ID_IN_SHM:
+ object_id = params[0].memref.buffer;
+ break;
+ default:
+ return TEE_ERROR_NOT_SUPPORTED;
+ }
res = TEE_OpenPersistentObject(params[2].value.a,
- object_id, object_id_size,
+ object_id, params[0].memref.size,
params[1].value.a, &o);
params[1].value.b = (uintptr_t)o;
- TEE_Free(object_id);
+
+ if (command == TA_STORAGE_CMD_OPEN)
+ TEE_Free(object_id);
return res;
}
-TEE_Result ta_storage_cmd_create(uint32_t param_types, TEE_Param params[4])
+TEE_Result ta_storage_cmd_create(uint32_t command,
+ uint32_t param_types, TEE_Param params[4])
{
TEE_Result res;
TEE_ObjectHandle o;
void *object_id;
- size_t object_id_size;
- TEE_ObjectHandle ref;
+ TEE_ObjectHandle ref_handle;
ASSERT_PARAM_TYPE(TEE_PARAM_TYPES
(TEE_PARAM_TYPE_MEMREF_INPUT,
@@ -82,32 +94,44 @@
TEE_PARAM_TYPE_VALUE_INPUT,
TEE_PARAM_TYPE_MEMREF_INPUT));
- object_id_size = params[0].memref.size;
- object_id = TEE_Malloc(object_id_size, 0);
- if (!object_id)
- return TEE_ERROR_OUT_OF_MEMORY;
+ switch (command) {
+ case TA_STORAGE_CMD_CREATE:
+ object_id = TEE_Malloc(params[0].memref.size, 0);
+ if (!object_id)
+ return TEE_ERROR_OUT_OF_MEMORY;
- TEE_MemMove(object_id, params[0].memref.buffer, object_id_size);
- ref = (TEE_ObjectHandle)(uintptr_t)params[2].value.a;
+ TEE_MemMove(object_id, params[0].memref.buffer,
+ params[0].memref.size);
+ break;
+ case TA_STORAGE_CMD_CREATE_ID_IN_SHM:
+ object_id = params[0].memref.buffer;
+ break;
+ default:
+ return TEE_ERROR_NOT_SUPPORTED;
+ }
+
+ ref_handle = (TEE_ObjectHandle)(uintptr_t)params[2].value.a;
res = TEE_CreatePersistentObject(params[2].value.b,
- object_id, object_id_size,
- params[1].value.a, ref,
+ object_id, params[0].memref.size,
+ params[1].value.a, ref_handle,
params[3].memref.buffer,
params[3].memref.size, &o);
+ if (command == TA_STORAGE_CMD_CREATE)
+ TEE_Free(object_id);
+
params[1].value.b = (uintptr_t)o;
- TEE_Free(object_id);
return res;
}
-TEE_Result ta_storage_cmd_create_overwrite(uint32_t param_types,
+TEE_Result ta_storage_cmd_create_overwrite(uint32_t command,
+ uint32_t param_types,
TEE_Param params[4])
{
TEE_Result res;
void *object_id;
- size_t object_id_size;
ASSERT_PARAM_TYPE(TEE_PARAM_TYPES
(TEE_PARAM_TYPE_MEMREF_INPUT,
@@ -115,19 +139,29 @@
TEE_PARAM_TYPE_NONE,
TEE_PARAM_TYPE_NONE));
- object_id_size = params[0].memref.size;
- object_id = TEE_Malloc(object_id_size, 0);
- if (!object_id)
- return TEE_ERROR_OUT_OF_MEMORY;
+ switch (command) {
+ case TA_STORAGE_CMD_CREATE_OVERWRITE:
+ object_id = TEE_Malloc(params[0].memref.size, 0);
+ if (!object_id)
+ return TEE_ERROR_OUT_OF_MEMORY;
- TEE_MemMove(object_id, params[0].memref.buffer, object_id_size);
+ TEE_MemMove(object_id, params[0].memref.buffer,
+ params[0].memref.size);
+ break;
+ case TA_STORAGE_CMD_CREATEOVER_ID_IN_SHM:
+ object_id = params[0].memref.buffer;
+ break;
+ default:
+ return TEE_ERROR_NOT_SUPPORTED;
+ }
res = TEE_CreatePersistentObject(params[1].value.a,
- object_id, object_id_size,
+ object_id, params[0].memref.size,
TEE_DATA_FLAG_OVERWRITE,
NULL, NULL, 0, NULL);
- TEE_Free(object_id);
+ if (command == TA_STORAGE_CMD_CREATE_OVERWRITE)
+ TEE_Free(object_id);
return res;
}
@@ -205,11 +239,11 @@
return TEE_SUCCESS;
}
-TEE_Result ta_storage_cmd_rename(uint32_t param_types, TEE_Param params[4])
+TEE_Result ta_storage_cmd_rename(uint32_t command, uint32_t param_types,
+ TEE_Param params[4])
{
TEE_ObjectHandle o = VAL2HANDLE(params[0].value.a);
void *object_id;
- size_t object_id_size;
TEE_Result res;
ASSERT_PARAM_TYPE(TEE_PARAM_TYPES
@@ -217,14 +251,26 @@
TEE_PARAM_TYPE_MEMREF_INPUT, TEE_PARAM_TYPE_NONE,
TEE_PARAM_TYPE_NONE));
- object_id_size = params[1].memref.size;
- object_id = TEE_Malloc(object_id_size, 0);
- if (!object_id)
- return TEE_ERROR_OUT_OF_MEMORY;
+ switch (command) {
+ case TA_STORAGE_CMD_RENAME:
+ object_id = TEE_Malloc(params[1].memref.size, 0);
+ if (!object_id)
+ return TEE_ERROR_OUT_OF_MEMORY;
- TEE_MemMove(object_id, params[1].memref.buffer, object_id_size);
- res = TEE_RenamePersistentObject(o, object_id, object_id_size);
- TEE_Free(object_id);
+ TEE_MemMove(object_id, params[1].memref.buffer,
+ params[1].memref.size);
+ break;
+ case TA_STORAGE_CMD_RENAME_ID_IN_SHM:
+ object_id = params[1].memref.buffer;
+ break;
+ default:
+ return TEE_ERROR_NOT_SUPPORTED;
+ }
+
+ res = TEE_RenamePersistentObject(o, object_id, params[1].memref.size);
+
+ if (command == TA_STORAGE_CMD_RENAME)
+ TEE_Free(object_id);
return res;
}
diff --git a/ta/storage/ta_entry.c b/ta/storage/ta_entry.c
index 7157550..edab315 100644
--- a/ta/storage/ta_entry.c
+++ b/ta/storage/ta_entry.c
@@ -71,7 +71,8 @@
switch (nCommandID) {
case TA_STORAGE_CMD_OPEN:
- return ta_storage_cmd_open(nParamTypes, pParams);
+ case TA_STORAGE_CMD_OPEN_ID_IN_SHM:
+ return ta_storage_cmd_open(nCommandID, nParamTypes, pParams);
case TA_STORAGE_CMD_CLOSE:
return ta_storage_cmd_close(nParamTypes, pParams);
@@ -83,10 +84,12 @@
return ta_storage_cmd_write(nParamTypes, pParams);
case TA_STORAGE_CMD_CREATE:
- return ta_storage_cmd_create(nParamTypes, pParams);
+ case TA_STORAGE_CMD_CREATE_ID_IN_SHM:
+ return ta_storage_cmd_create(nCommandID, nParamTypes, pParams);
case TA_STORAGE_CMD_CREATE_OVERWRITE:
- return ta_storage_cmd_create_overwrite(nParamTypes, pParams);
+ case TA_STORAGE_CMD_CREATEOVER_ID_IN_SHM:
+ return ta_storage_cmd_create_overwrite(nCommandID, nParamTypes, pParams);
case TA_STORAGE_CMD_SEEK:
return ta_storage_cmd_seek(nParamTypes, pParams);
@@ -95,7 +98,8 @@
return ta_storage_cmd_unlink(nParamTypes, pParams);
case TA_STORAGE_CMD_RENAME:
- return ta_storage_cmd_rename(nParamTypes, pParams);
+ case TA_STORAGE_CMD_RENAME_ID_IN_SHM:
+ return ta_storage_cmd_rename(nCommandID, nParamTypes, pParams);
case TA_STORAGE_CMD_TRUNC:
return ta_storage_cmd_trunc(nParamTypes, pParams);