summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: d244a2a)
raw | patch | inline | side by side (parent: d244a2a)
author | Etienne Carriere <etienne.carriere@linaro.org> | |
Thu, 26 Apr 2018 12:20:35 +0000 (14:20 +0200) | ||
committer | Jérôme Forissier <jerome.forissier@linaro.org> | |
Thu, 26 Apr 2018 12:35:22 +0000 (14:35 +0200) |
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>
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>
index 336ea142b441536ff09574528e2123966fff736c..c0e8e904fe3db3a1a3f9569bb333f904e3768750 100644 (file)
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,
"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)");
index c5e8d3847391299db11782c54feba1775a62d28d..f0f892c81532e29dc04c3ae75ad8863adbe716de 100644 (file)
--- a/ta/include/ta_storage.h
+++ b/ta/include/ta_storage.h
#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*/
index d4a4ddc91ad7a1c1f82d35f819c6b86054bf8cb3..b0a1575fe5df121a082857c74e534f829f3acadd 100644 (file)
#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 6d9759767f497406c3ba0cdbf8e7395fb78e5def..c61d548ab80d5c620da6e2cac0168ab13bb243e1 100644 (file)
--- a/ta/storage/storage.c
+++ b/ta/storage/storage.c
*/
#include "storage.h"
+#include "ta_storage.h"
#include <tee_api.h>
#include <trace.h>
#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,
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;
-
- TEE_MemMove(object_id, params[0].memref.buffer, object_id_size);
+ 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,
+ 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,
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,
+ 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;
+ }
- TEE_MemMove(object_id, params[0].memref.buffer, object_id_size);
- ref = (TEE_ObjectHandle)(uintptr_t)params[2].value.a;
+ 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,
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;
-
- TEE_MemMove(object_id, params[0].memref.buffer, object_id_size);
+ 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,
+ 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;
}
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
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,
+ 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);
- TEE_MemMove(object_id, params[1].memref.buffer, object_id_size);
- res = TEE_RenamePersistentObject(o, object_id, object_id_size);
- TEE_Free(object_id);
+ 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 71575507efbeac59776dc923bbbfe0b70cf3faaa..edab315797328344461d5f6f686606b801d37529 100644 (file)
--- a/ta/storage/ta_entry.c
+++ b/ta/storage/ta_entry.c
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);
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);
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);