Attest: Fix unaligned access issue
Use memory primitives to copy data instead of
pointer casting and dereferencing when memory
address can be unaligned.
Change-Id: I71ed1a63a1a8c8699a1680c82b934094e8ab5db3
Signed-off-by: Tamas Ban <tamas.ban@arm.com>
diff --git a/bl2/src/boot_record.c b/bl2/src/boot_record.c
index e24d2c4..21c1904 100644
--- a/bl2/src/boot_record.c
+++ b/bl2/src/boot_record.c
@@ -280,7 +280,8 @@
* returns with error: SHARED_MEMORY_OVERWRITE
*/
for (; offset < tlv_end; offset += tlv_entry.tlv_len) {
- tlv_entry = *((struct shared_data_tlv_entry *)offset);
+ /* Create local copy to avoid unaligned access */
+ memcpy(&tlv_entry, (const void *)offset, SHARED_DATA_ENTRY_HEADER_SIZE);
if (GET_MAJOR(tlv_entry.tlv_type) == major_type &&
GET_MINOR(tlv_entry.tlv_type) == minor_type) {
return SHARED_MEMORY_OVERWRITE;
diff --git a/secure_fw/core/tfm_boot_data.c b/secure_fw/core/tfm_boot_data.c
index e742072..9d65960 100644
--- a/secure_fw/core/tfm_boot_data.c
+++ b/secure_fw/core/tfm_boot_data.c
@@ -57,13 +57,13 @@
void tfm_core_get_boot_data_handler(uint32_t args[])
{
uint8_t tlv_major = (uint8_t)args[0];
- uint8_t *ptr = (uint8_t *)args[1];
+ uint8_t *buf_start = (uint8_t *)args[1];
uint16_t buf_size = (uint16_t)args[2];
- uint8_t *buf_start = ptr;
+ uint8_t *ptr;
uint32_t running_partition_idx =
tfm_spm_partition_get_running_partition_idx();
struct shared_data_tlv_header *tlv_header;
- struct shared_data_tlv_entry *tlv_entry;
+ struct shared_data_tlv_entry tlv_entry;
uintptr_t tlv_end, offset;
uint32_t res;
@@ -71,9 +71,9 @@
* by the partition
*/
res = tfm_core_check_buffer_access(running_partition_idx,
- (void*)buf_start,
+ (void *)buf_start,
buf_size,
- 2);
+ 2); /* Check 4 bytes alignment */
if (!res) {
/* Not in accessible range, return error */
args[0] = TFM_ERROR_INVALID_PARAMETER;
@@ -97,28 +97,31 @@
args[0] = TFM_ERROR_INVALID_PARAMETER;
return;
} else {
- tlv_header = (struct shared_data_tlv_header *)ptr;
+ tlv_header = (struct shared_data_tlv_header *)buf_start;
tlv_header->tlv_magic = SHARED_DATA_TLV_INFO_MAGIC;
tlv_header->tlv_tot_len = SHARED_DATA_HEADER_SIZE;
- ptr += SHARED_DATA_HEADER_SIZE;
+ ptr = (uint8_t *)tlv_header + SHARED_DATA_HEADER_SIZE;
}
/* Iterates over the TLV section and copy TLVs with requested major
* type to the provided buffer.
*/
- for(; offset < tlv_end; offset += tlv_entry->tlv_len) {
- tlv_entry = (struct shared_data_tlv_entry *)offset;
- if (GET_MAJOR(tlv_entry->tlv_type) == tlv_major) {
+ for (; offset < tlv_end; offset += tlv_entry.tlv_len) {
+ /* Create local copy to avoid unaligned access */
+ tfm_memcpy(&tlv_entry,
+ (const void *)offset,
+ SHARED_DATA_ENTRY_HEADER_SIZE);
+ if (GET_MAJOR(tlv_entry.tlv_type) == tlv_major) {
/* Check buffer overflow */
- if ((ptr - buf_start + tlv_entry->tlv_len) > buf_size) {
+ if ((ptr - buf_start + tlv_entry.tlv_len) > buf_size) {
args[0] = TFM_ERROR_INVALID_PARAMETER;
return;
}
- tfm_memcpy(ptr, (const void *)tlv_entry, tlv_entry->tlv_len);
+ tfm_memcpy(ptr, (const void *)offset, tlv_entry.tlv_len);
- ptr += tlv_entry->tlv_len;
- tlv_header->tlv_tot_len += tlv_entry->tlv_len;
+ ptr += tlv_entry.tlv_len;
+ tlv_header->tlv_tot_len += tlv_entry.tlv_len;
}
}
args[0] = TFM_SUCCESS;
diff --git a/secure_fw/services/initial_attestation/attestation_core.c b/secure_fw/services/initial_attestation/attestation_core.c
index b2cd01c..245c1cf 100644
--- a/secure_fw/services/initial_attestation/attestation_core.c
+++ b/secure_fw/services/initial_attestation/attestation_core.c
@@ -18,6 +18,7 @@
#include "attest_token.h"
#include "attest_eat_defines.h"
#include "t_cose_defines.h"
+#include "tfm_memory_utils.h"
#define MAX_BOOT_STATUS 512
@@ -41,10 +42,7 @@
* the service related data from shared area.
*/
-/* FixMe: Enforcement of 4 byte alignment can be removed as soon as memory type
- * is configured in the MPU to be normal, instead of device, which
- * prohibits unaligned access.
- */
+/* Enforcement of 4 byte alignment, which is checked by TF-M SPM */
__attribute__ ((aligned(4)))
static uint8_t boot_status[MAX_BOOT_STATUS];
@@ -84,6 +82,11 @@
* \brief Static function to convert a pointer and size info to unsigned
* integer number. Max 32bits unsigned integers are supported.
*
+ * This implementation assumes that the endianness of the sender and receiver
+ * of the data is the same because they are actually running on the same CPU
+ * instance. If this assumption is not true than this function must be
+ * refactored accordingly.
+ *
* \param[in] int_ptr Pointer to the unsigned integer
* \param[in] len Size of the unsigned integers in bytes
* \param[in] value Pointer where to store the converted value
@@ -94,15 +97,20 @@
size_t len,
uint32_t *value)
{
+ uint16_t uint16;
+
switch (len) {
case 1:
*value = (uint32_t)(*(uint8_t *)(int_ptr));
break;
case 2:
- *value = (uint32_t)(*(uint16_t *)(int_ptr));
+ /* Avoid unaligned access */
+ tfm_memcpy(&uint16, int_ptr, sizeof(uint16));
+ *value = (uint32_t)uint16;
break;
case 4:
- *value = (uint32_t)(*(uint32_t *)(int_ptr));
+ /* Avoid unaligned access */
+ tfm_memcpy(value, int_ptr, sizeof(uint32_t));
break;
default:
return -1;
@@ -134,7 +142,7 @@
uint8_t **tlv_ptr)
{
struct shared_data_tlv_header *tlv_header;
- struct shared_data_tlv_entry *tlv_entry;
+ struct shared_data_tlv_entry tlv_entry;
uint8_t *tlv_end;
uint8_t *tlv_curr;
@@ -144,25 +152,26 @@
}
/* Get the boundaries of TLV section where to lookup*/
- tlv_end = boot_status + tlv_header->tlv_tot_len;
+ tlv_end = (uint8_t *)boot_status + tlv_header->tlv_tot_len;
if (*tlv_ptr == NULL) {
/* At first call set to the beginning of the TLV section */
- tlv_curr = boot_status + SHARED_DATA_HEADER_SIZE;
+ tlv_curr = (uint8_t *)boot_status + SHARED_DATA_HEADER_SIZE;
} else {
/* Any subsequent call set to the next TLV entry */
- tlv_entry = (struct shared_data_tlv_entry *)(*tlv_ptr);
- tlv_curr = (*tlv_ptr) + tlv_entry->tlv_len;
+ tfm_memcpy(&tlv_entry, *tlv_ptr, SHARED_DATA_ENTRY_HEADER_SIZE);
+ tlv_curr = (*tlv_ptr) + tlv_entry.tlv_len;
}
/* Iterates over the TLV section and returns the address and size of TLVs
* with requested module identifier
*/
- for (; tlv_curr < tlv_end; tlv_curr += tlv_entry->tlv_len) {
- tlv_entry = (struct shared_data_tlv_entry *)tlv_curr;
- if (GET_IAS_MODULE(tlv_entry->tlv_type) == module) {
- *claim = GET_IAS_CLAIM(tlv_entry->tlv_type);
- *tlv_ptr = (uint8_t *)tlv_entry;
- *tlv_len = tlv_entry->tlv_len;
+ for (; tlv_curr < tlv_end; tlv_curr += tlv_entry.tlv_len) {
+ /* Create local copy to avoid unaligned access */
+ tfm_memcpy(&tlv_entry, tlv_curr, SHARED_DATA_ENTRY_HEADER_SIZE);
+ if (GET_IAS_MODULE(tlv_entry.tlv_type) == module) {
+ *claim = GET_IAS_CLAIM(tlv_entry.tlv_type);
+ *tlv_ptr = tlv_curr;
+ *tlv_len = tlv_entry.tlv_len;
return 1;
}
}
@@ -294,16 +303,20 @@
uint8_t *tlv_address,
uint32_t nested_map)
{
- struct shared_data_tlv_entry *tlv_entry =
- (struct shared_data_tlv_entry *) tlv_address;
- uint16_t tlv_len = tlv_entry->tlv_len;
- uint8_t tlv_id = GET_IAS_CLAIM(tlv_entry->tlv_type);
+ struct shared_data_tlv_entry tlv_entry;
+ uint16_t tlv_len;
+ uint8_t tlv_id;
uint8_t *tlv_ptr = tlv_address;
int32_t found = 1;
struct useful_buf_c claim_value;
enum psa_attest_err_t res;
QCBOREncodeContext *cbor_encode_ctx;
+ /* Create local copy to avoid unaligned access */
+ tfm_memcpy(&tlv_entry, tlv_address, SHARED_DATA_ENTRY_HEADER_SIZE);
+ tlv_len = tlv_entry.tlv_len;
+ tlv_id = GET_IAS_CLAIM(tlv_entry.tlv_type);
+
cbor_encode_ctx = attest_token_borrow_cbor_cntxt(token_ctx);
/* Open nested map for SW component measurement claims */
@@ -357,16 +370,20 @@
uint32_t module,
uint8_t *tlv_address)
{
- struct shared_data_tlv_entry *tlv_entry =
- (struct shared_data_tlv_entry *) tlv_address;
- uint16_t tlv_len = tlv_entry->tlv_len;
- uint8_t tlv_id = GET_IAS_CLAIM(tlv_entry->tlv_type);
+ struct shared_data_tlv_entry tlv_entry;
+ uint16_t tlv_len;
+ uint8_t tlv_id;
uint8_t *tlv_ptr = tlv_address;
int32_t found = 1;
uint32_t measurement_claim_cnt = 0;
struct useful_buf_c claim_value;
QCBOREncodeContext *cbor_encode_ctx;
+ /* Create local copy to avoid unaligned access */
+ tfm_memcpy(&tlv_entry, tlv_address, SHARED_DATA_ENTRY_HEADER_SIZE);
+ tlv_len = tlv_entry.tlv_len;
+ tlv_id = GET_IAS_CLAIM(tlv_entry.tlv_type);
+
/* Open map which stores claims belong to a SW component */
cbor_encode_ctx = attest_token_borrow_cbor_cntxt(token_ctx);
QCBOREncode_OpenMap(cbor_encode_ctx);