feat(memory share): check memory region desc values
This patch introduces a helper function to check that the values
provided in the memory region descriptor are valid and safe.
Offset values are within safe bounds, receiver count will not
cause overflows and reserved fields are 0.
Signed-off-by: Daniel Boulby <daniel.boulby@arm.com>
Change-Id: If3e751cf80cd5a0ecbcc77245a91daac50465a5c
diff --git a/inc/hf/ffa_memory.h b/inc/hf/ffa_memory.h
index 8187e6f..7d7ccac 100644
--- a/inc/hf/ffa_memory.h
+++ b/inc/hf/ffa_memory.h
@@ -13,6 +13,11 @@
#include "vmapi/hf/ffa.h"
+bool ffa_memory_region_sanity_check(struct ffa_memory_region *memory_region,
+ uint32_t ffa_version,
+ uint32_t fragment_length,
+ bool send_transaction);
+
struct ffa_value ffa_memory_send(struct vm_locked from_locked,
struct ffa_memory_region *memory_region,
uint32_t memory_share_length,
diff --git a/inc/vmapi/hf/ffa.h b/inc/vmapi/hf/ffa.h
index 8b61fbe..4ab3592 100644
--- a/inc/vmapi/hf/ffa.h
+++ b/inc/vmapi/hf/ffa.h
@@ -23,6 +23,17 @@
#define FFA_VERSION_COMPILED \
MAKE_FFA_VERSION(FFA_VERSION_MAJOR, FFA_VERSION_MINOR)
+/**
+ * Check major versions are equal and the minor version of the caller is
+ * less than or equal to the minor version of the callee.
+ */
+#define FFA_VERSIONS_ARE_COMPATIBLE(v_caller, v_callee) \
+ ((((v_caller >> FFA_VERSION_MAJOR_OFFSET) & FFA_VERSION_MAJOR_MASK) == \
+ ((v_callee >> FFA_VERSION_MAJOR_OFFSET) & \
+ FFA_VERSION_MAJOR_MASK)) && \
+ (((v_caller >> FFA_VERSION_MINOR_OFFSET) & FFA_VERSION_MINOR_MASK) <= \
+ ((v_callee >> FFA_VERSION_MINOR_OFFSET) & FFA_VERSION_MINOR_MASK)))
+
/* clang-format off */
#define FFA_LOW_32_ID 0x84000060
@@ -1128,6 +1139,13 @@
};
/**
+ * Returns the first FF-A version that matches the memory access descriptor
+ * size.
+ */
+uint32_t ffa_version_from_memory_access_desc_size(
+ uint32_t memory_access_desc_size);
+
+/**
* To maintain forwards compatability we can't make assumptions about the size
* of the endpoint memory access descriptor so provide a helper function
* to get a receiver from the receiver array using the memory access descriptor
diff --git a/src/api.c b/src/api.c
index b40880a..0fb942b 100644
--- a/src/api.c
+++ b/src/api.c
@@ -2981,24 +2981,6 @@
memory_region_v1_0 = (struct ffa_memory_region_v1_0 *)allocated;
- if (memory_region_v1_0->reserved_0 != 0U ||
- memory_region_v1_0->reserved_1 != 0U) {
- dlog_verbose(
- "Memory region descriptor reserved fields must be "
- "0.\n");
- return ffa_error(FFA_INVALID_PARAMETERS);
- }
-
- /* This should also prevent over flows. */
- if (memory_region_v1_0->receiver_count > MAX_MEM_SHARE_RECIPIENTS) {
- dlog_verbose(
- "Max number of recipients supported is %u "
- "specified %u\n",
- MAX_MEM_SHARE_RECIPIENTS,
- memory_region_v1_0->receiver_count);
- return ffa_error(FFA_INVALID_PARAMETERS);
- }
-
receivers_length = sizeof(struct ffa_memory_access_v1_0) *
memory_region_v1_0->receiver_count;
receivers_end = sizeof(struct ffa_memory_region) + receivers_length;
@@ -3010,50 +2992,6 @@
composite_offset_v1_0 =
memory_region_v1_0->receivers[0].composite_memory_region_offset;
- if (!send_transaction) {
- if (composite_offset_v1_0 != 0) {
- dlog_verbose(
- "Composite offset memory region descriptor "
- "offset must be 0 for retrieve requests. "
- "Currently %d\n",
- composite_offset_v1_0);
- return ffa_error(FFA_INVALID_PARAMETERS);
- }
- } else {
- bool comp_offset_is_zero = composite_offset_v1_0 == 0U;
- bool comp_offset_lt_transaction_descriptor_size =
- composite_offset_v1_0 <
- sizeof(struct ffa_memory_region_v1_0) +
- receivers_length;
- bool comp_offset_with_comp_gt_fragment_length =
- composite_offset_v1_0 +
- sizeof(struct ffa_composite_memory_region) >
- *fragment_length;
- if (comp_offset_is_zero ||
- comp_offset_lt_transaction_descriptor_size ||
- comp_offset_with_comp_gt_fragment_length) {
- dlog_verbose(
- "Invalid composite memory region descriptor "
- "offset for send transaction %d\n",
- composite_offset_v1_0);
- return ffa_error(FFA_INVALID_PARAMETERS);
- }
- }
-
- for (uint32_t i = 1; i < memory_region_v1_0->receiver_count; i++) {
- const uint32_t current_offset =
- memory_region_v1_0->receivers[i]
- .composite_memory_region_offset;
-
- if (current_offset != composite_offset_v1_0) {
- dlog_verbose(
- "Composite offset %x differs from %x in index "
- "%u\n",
- composite_offset_v1_0, current_offset, i);
- return ffa_error(FFA_INVALID_PARAMETERS);
- }
- }
-
/* Determine the composite offset for v1.1 descriptor. */
if (send_transaction) {
fragment_constituents_size =
@@ -3224,6 +3162,12 @@
memcpy_s(allocated_entry, MM_PPOOL_ENTRY_SIZE, from_msg,
fragment_length);
+ if (!ffa_memory_region_sanity_check(allocated_entry, ffa_version,
+ fragment_length, true)) {
+ ret = ffa_error(FFA_INVALID_PARAMETERS);
+ goto out;
+ }
+
ret = api_ffa_memory_transaction_descriptor_v1_1_from_v1_0(
allocated_entry, &fragment_length, &length, ffa_version, true);
if (ret.func != FFA_SUCCESS_32) {
@@ -3232,18 +3176,6 @@
memory_region = allocated_entry;
- if (memory_region->memory_access_desc_size <
- sizeof(struct ffa_memory_access_v1_0)) {
- dlog_verbose(
- "Invalid memory access descritor size %d must be "
- "at least as large as the v1_0 memory access array "
- "%d\n",
- memory_region->memory_access_desc_size,
- sizeof(struct ffa_memory_access_v1_0));
- ret = ffa_error(FFA_INVALID_PARAMETERS);
- goto out;
- }
-
if (fragment_length < sizeof(struct ffa_memory_region) +
memory_region->memory_access_desc_size) {
dlog_verbose(
@@ -3279,16 +3211,6 @@
goto out;
}
- if (memory_region->receiver_count > MAX_MEM_SHARE_RECIPIENTS) {
- dlog_verbose(
- "Max number of recipients supported is %u "
- "specified %u\n",
- MAX_MEM_SHARE_RECIPIENTS,
- memory_region->receiver_count);
- ret = ffa_error(FFA_INVALID_PARAMETERS);
- goto out;
- }
-
/*
* Ensure that the receiver VM exists and isn't the same as the sender.
* If there is a receiver from the other world, track it for later
@@ -3410,6 +3332,12 @@
goto out;
}
+ if (!ffa_memory_region_sanity_check(retrieve_msg, ffa_version,
+ fragment_length, false)) {
+ ret = ffa_error(FFA_INVALID_PARAMETERS);
+ goto out;
+ }
+
/*
* If required, transform the retrieve request to FF-A v1.1.
*/
diff --git a/src/ffa_memory.c b/src/ffa_memory.c
index 58f39ae..546d188 100644
--- a/src/ffa_memory.c
+++ b/src/ffa_memory.c
@@ -445,6 +445,215 @@
return (struct ffa_value){.func = FFA_SUCCESS_32};
}
+uint32_t ffa_version_from_memory_access_desc_size(
+ uint32_t memory_access_desc_size)
+{
+ switch (memory_access_desc_size) {
+ /*
+ * v1.0 and v1.1 memory access descriptors are the same size however
+ * v1.1 is the first version to include the memory access descriptor
+ * size field so return v1.1.
+ */
+ case sizeof(struct ffa_memory_access):
+ return MAKE_FFA_VERSION(1, 1);
+ }
+ return 0;
+}
+
+/**
+ * Check if the receivers size and offset given is valid for the senders
+ * FF-A version.
+ */
+static bool receiver_size_and_offset_valid_for_version(
+ uint32_t receivers_size, uint32_t receivers_offset,
+ uint32_t ffa_version)
+{
+ /*
+ * Check that the version that the memory access descriptor size belongs
+ * to is compatible with the FF-A version we believe the sender to be.
+ */
+ uint32_t expected_ffa_version =
+ ffa_version_from_memory_access_desc_size(receivers_size);
+ if (!FFA_VERSIONS_ARE_COMPATIBLE(expected_ffa_version, ffa_version)) {
+ return false;
+ }
+
+ /*
+ * Check the receivers_offset matches the version we found from
+ * memory access descriptor size.
+ */
+ switch (expected_ffa_version) {
+ case MAKE_FFA_VERSION(1, 1):
+ return receivers_offset == sizeof(struct ffa_memory_region);
+ default:
+ return false;
+ }
+}
+
+/**
+ * Check the values set for fields in the memory region are valid and safe.
+ * Offset values are within safe bounds, receiver count will not cause overflows
+ * and reserved fields are 0.
+ */
+bool ffa_memory_region_sanity_check(struct ffa_memory_region *memory_region,
+ uint32_t ffa_version,
+ uint32_t fragment_length,
+ bool send_transaction)
+{
+ uint32_t receiver_count;
+ struct ffa_memory_access *receiver;
+ uint32_t composite_offset_0;
+
+ if (ffa_version == MAKE_FFA_VERSION(1, 0)) {
+ struct ffa_memory_region_v1_0 *memory_region_v1_0 =
+ (struct ffa_memory_region_v1_0 *)memory_region;
+ /* Check the reserved fields are 0. */
+ if (memory_region_v1_0->reserved_0 != 0 ||
+ memory_region_v1_0->reserved_1 != 0) {
+ dlog_verbose("Reserved fields must be 0.\n");
+ return false;
+ }
+
+ receiver_count = memory_region_v1_0->receiver_count;
+ } else {
+ uint32_t receivers_size =
+ memory_region->memory_access_desc_size;
+ uint32_t receivers_offset = memory_region->receivers_offset;
+
+ /* Check the reserved field is 0. */
+ if (memory_region->reserved[0] != 0 ||
+ memory_region->reserved[1] != 0 ||
+ memory_region->reserved[2] != 0) {
+ dlog_verbose("Reserved fields must be 0.\n");
+ return false;
+ }
+
+ /*
+ * Check memory_access_desc_size matches the size of the struct
+ * for the senders FF-A version.
+ */
+ if (!receiver_size_and_offset_valid_for_version(
+ receivers_size, receivers_offset, ffa_version)) {
+ dlog_verbose(
+ "Invalid memory access descriptor size %d, "
+ " or receiver offset %d, "
+ "for FF-A version %#x\n",
+ receivers_size, receivers_offset, ffa_version);
+ return false;
+ }
+
+ receiver_count = memory_region->receiver_count;
+ }
+
+ /* Check receiver count is not too large. */
+ if (receiver_count > MAX_MEM_SHARE_RECIPIENTS) {
+ dlog_verbose(
+ "Max number of recipients supported is %u "
+ "specified %u\n",
+ MAX_MEM_SHARE_RECIPIENTS, receiver_count);
+ return false;
+ }
+
+ /* Check values in the memory access descriptors. */
+ /*
+ * The composite offset values must be the same for all recievers so
+ * check the first one is valid and then they are all the same.
+ */
+ receiver = ffa_version == MAKE_FFA_VERSION(1, 0)
+ ? (struct ffa_memory_access *)&(
+ (struct ffa_memory_region_v1_0 *)
+ memory_region)
+ ->receivers[0]
+ : ffa_memory_region_get_receiver(memory_region, 0);
+ assert(receiver != NULL);
+ composite_offset_0 = receiver->composite_memory_region_offset;
+
+ if (!send_transaction) {
+ if (composite_offset_0 != 0) {
+ dlog_verbose(
+ "Composite offset memory region descriptor "
+ "offset must be 0 for retrieve requests. "
+ "Currently %d",
+ composite_offset_0);
+ return false;
+ }
+ } else {
+ bool comp_offset_is_zero = composite_offset_0 == 0U;
+ bool comp_offset_lt_transaction_descriptor_size =
+ composite_offset_0 <
+ (sizeof(struct ffa_memory_region) +
+ (uint32_t)(memory_region->memory_access_desc_size *
+ memory_region->receiver_count));
+ bool comp_offset_with_comp_gt_fragment_length =
+ composite_offset_0 +
+ sizeof(struct ffa_composite_memory_region) >
+ fragment_length;
+ if (comp_offset_is_zero ||
+ comp_offset_lt_transaction_descriptor_size ||
+ comp_offset_with_comp_gt_fragment_length) {
+ dlog_verbose(
+ "Invalid composite memory region descriptor "
+ "offset for send transaction %u\n",
+ composite_offset_0);
+ return false;
+ }
+ }
+
+ for (int i = 0; i < memory_region->receiver_count; i++) {
+ uint32_t composite_offset;
+
+ if (ffa_version == MAKE_FFA_VERSION(1, 0)) {
+ struct ffa_memory_region_v1_0 *memory_region_v1_0 =
+ (struct ffa_memory_region_v1_0 *)memory_region;
+
+ struct ffa_memory_access_v1_0 *receiver_v1_0 =
+ &memory_region_v1_0->receivers[i];
+ /* Check reserved fields are 0 */
+ if (receiver_v1_0->reserved_0 != 0) {
+ dlog_verbose(
+ "Reserved field in the memory access "
+ " descriptor must be zero "
+ " Currently reciever %d has a reserved "
+ " field with a value of %d\n",
+ i, receiver_v1_0->reserved_0);
+ return false;
+ }
+ /*
+ * We can cast to the current version receiver as the
+ * remaining fields we are checking have the same
+ * offsets for all versions since memory access
+ * descriptors are forwards compatible.
+ */
+ receiver = (struct ffa_memory_access *)receiver_v1_0;
+ } else {
+ receiver = ffa_memory_region_get_receiver(memory_region,
+ i);
+ assert(receiver != NULL);
+
+ if (receiver->reserved_0 != 0) {
+ dlog_verbose(
+ "Reserved field in the memory access "
+ " descriptor must be zero "
+ " Currently reciever %d has a reserved "
+ " field with a value of %d\n",
+ i, receiver->reserved_0);
+ return false;
+ }
+ }
+
+ /* Check composite offset values are equal for all receivers. */
+ composite_offset = receiver->composite_memory_region_offset;
+ if (composite_offset != composite_offset_0) {
+ dlog_verbose(
+ "Composite offset %x differs from %x in index "
+ "%u\n",
+ composite_offset, composite_offset_0);
+ return false;
+ }
+ }
+ return true;
+}
+
/**
* Verify that all pages have the same mode, that the starting mode
* constitutes a valid state and obtain the next mode to apply