Core: tfm_spm_get_msg_from_handle to check handle
Add extra checks to tfm_spm_get_msg_from_handle to validate message
handler:
- Check the handler is allocated from the pool
- Use magic to signal that a handler is active
Change-Id: Ic7b13cc1d61ae48e6112864bb2a1ce2059247e65
Signed-off-by: Mate Toth-Pal <mate.toth-pal@arm.com>
diff --git a/secure_fw/core/ipc/include/tfm_pools.h b/secure_fw/core/ipc/include/tfm_pools.h
index 1072f78..64be3c4 100644
--- a/secure_fw/core/ipc/include/tfm_pools.h
+++ b/secure_fw/core/ipc/include/tfm_pools.h
@@ -7,6 +7,12 @@
#ifndef __TFM_POOLS_H__
#define __TFM_POOLS_H__
+#include <stdbool.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
/*
* Resource pool - few known size resources allocation/free is required,
* so pool is more applicable than heap.
@@ -17,15 +23,16 @@
* [ Pool Instance ] + N * [ Pool Chunks ]
*/
struct tfm_pool_chunk_t {
- struct tfm_list_node_t list; /* Chunk list */
- void *pool; /* Point to the parent pool */
- uint8_t data[0]; /* Data indicator */
+ struct tfm_list_node_t list; /* Chunk list */
+ void *pool; /* Point to the parent pool */
+ uint8_t data[0]; /* Data indicator */
};
struct tfm_pool_instance_t {
- size_t chunksz; /* Chunks size of pool member */
- struct tfm_list_node_t chunks_list; /* Chunk list head in pool */
- struct tfm_pool_chunk_t chunks[0]; /* Data indicator */
+ size_t chunksz; /* Chunks size of pool member */
+ size_t chunk_count; /* A number of chunks in the pool */
+ struct tfm_list_node_t chunks_list; /* Chunk list head in pool */
+ struct tfm_pool_chunk_t chunks[0]; /* Data indicator */
};
/*
@@ -82,4 +89,21 @@
*/
void tfm_pool_free(void *ptr);
+/**
+ * \brief Checks whether a pointer points to a chunk data in the pool.
+ *
+ * \param[in] pool Pointer to memory pool declared by
+ * \ref TFM_POOL_DECLARE.
+ * \param[in] data The pointer to check.
+ *
+ * \retval true Data is a chunk data in the pool.
+ * \retval false Data is not a chunk data in the pool.
+ */
+bool is_valid_chunk_data_in_pool(struct tfm_pool_instance_t *pool,
+ uint8_t *data);
+
+#ifdef __cplusplus
+}
#endif
+
+#endif /* __TFM_POOLS_H__ */
diff --git a/secure_fw/core/ipc/tfm_pools.c b/secure_fw/core/ipc/tfm_pools.c
index 1549460..6ab56af 100644
--- a/secure_fw/core/ipc/tfm_pools.c
+++ b/secure_fw/core/ipc/tfm_pools.c
@@ -50,6 +50,7 @@
/* Prepare instance and insert to pool list */
pool->chunksz = chunksz;
+ pool->chunk_count = num;
return IPC_SUCCESS;
}
@@ -85,3 +86,28 @@
pool = (struct tfm_pool_instance_t *)pchunk->pool;
tfm_list_add_tail(&pool->chunks_list, &pchunk->list);
}
+
+bool is_valid_chunk_data_in_pool(struct tfm_pool_instance_t *pool,
+ uint8_t *data)
+{
+ const uintptr_t chunks_start = (uintptr_t)(pool->chunks);
+ const size_t chunks_size = pool->chunksz + sizeof(struct tfm_pool_chunk_t);
+ const size_t chunk_count = pool->chunk_count;
+ const uintptr_t chunks_end = chunks_start + chunks_size * chunk_count;
+ uintptr_t pool_chunk_address = 0;
+
+ /* Check that the message was allocated from the pool. */
+ if ((uintptr_t)data < chunks_start || (uintptr_t)data >= chunks_end) {
+ return false;
+ }
+
+ pool_chunk_address =
+ (uint32_t)TFM_GET_CONTAINER_PTR(data, struct tfm_pool_chunk_t, data);
+
+ /* Make sure that the chunk containing the message is aligned on */
+ /* chunk boundary in the pool. */
+ if ((pool_chunk_address - chunks_start) % chunks_size != 0) {
+ return false;
+ }
+ return true;
+}
diff --git a/secure_fw/spm/spm_api_ipc.c b/secure_fw/spm/spm_api_ipc.c
index 70980f2..98f7a0e 100644
--- a/secure_fw/spm/spm_api_ipc.c
+++ b/secure_fw/spm/spm_api_ipc.c
@@ -90,6 +90,9 @@
tfm_panic();
}
+ /* Clear magic as the handler is not used anymore */
+ p_handle->internal_msg.magic = 0;
+
/* Remove node from handle list */
tfm_list_del_node(&p_handle->list);
@@ -241,34 +244,49 @@
struct tfm_msg_body_t *tfm_spm_get_msg_from_handle(psa_handle_t msg_handle)
{
/*
- * There may be one error handle passed by the caller in two conditions:
- * 1. Not a valid message handle.
- * 2. Handle between different Partitions. Partition A passes one handle
- * belong to other Partitions and tries to access other's data.
- * So, need do necessary checking to prevent those conditions.
+ * The message handler passed by the caller is considered invalid in the
+ * following cases:
+ * 1. Not a valid message handle. (The address of a message is not the
+ * address of a possible handle from the pool
+ * 2. Handle not belongs to the caller partition (The handle is either
+ * unused, or owned by anither partition)
+ * Check the conditions above
*/
+ struct tfm_conn_handle_t *connection_handle_address;
struct tfm_msg_body_t *msg;
uint32_t partition_id;
msg = (struct tfm_msg_body_t *)msg_handle;
- if (!msg) {
+
+ connection_handle_address =
+ TFM_GET_CONTAINER_PTR(msg, struct tfm_conn_handle_t, internal_msg);
+
+ if (is_valid_chunk_data_in_pool(
+ conn_handle_pool, (uint8_t *)connection_handle_address) != 1) {
return NULL;
}
/*
- * FixMe: For condition 1: using a magic number to define it's a message.
- * It needs to be an enhancement to check the handle belong to service.
+ * Check that the magic number is correct. This proves that the message
+ * structure contains an active message.
*/
if (msg->magic != TFM_MSG_MAGIC) {
return NULL;
}
- /* For condition 2: check if the partition ID is same */
+ /* Check that the running partition owns the message */
partition_id = tfm_spm_partition_get_running_partition_id();
if (partition_id != msg->service->partition->static_data->partition_id) {
return NULL;
}
+ /*
+ * FixMe: For condition 1 it should be checked whether the message belongs
+ * to the service. Skipping this check isn't a security risk as even if the
+ * message belongs to another service, the handle belongs to the calling
+ * partition.
+ */
+
return msg;
}