Hardening FF-A RPC caller
Fix buffer overflow in FF-A RPC calls with large (>~4k) request size.
Add additional error checks. Refactor file to meet checkpatch
requirements.
Signed-off-by: Imre Kis <imre.kis@arm.com>
Change-Id: I5b231cce4a5e79ace074ce9288c176d88b76aaaa
diff --git a/components/rpc/ffarpc/caller/sp/ffarpc_caller.c b/components/rpc/ffarpc/caller/sp/ffarpc_caller.c
index dabcd90..981908e 100644
--- a/components/rpc/ffarpc/caller/sp/ffarpc_caller.c
+++ b/components/rpc/ffarpc/caller/sp/ffarpc_caller.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2020-2021, Arm Limited and Contributors. All rights reserved.
+ * Copyright (c) 2020-2022, Arm Limited and Contributors. All rights reserved.
*
* SPDX-License-Identifier: BSD-3-Clause
*/
@@ -25,33 +25,29 @@
struct ffarpc_caller *this_context = (struct ffarpc_caller *)context;
rpc_call_handle handle = NULL;
- if (req_buf == NULL) {
- EMSG("call_begin(): invalid arguments");
+ if (context == NULL || req_buf == NULL) {
+ EMSG("invalid arguments");
goto out;
}
if (this_context->is_call_transaction_in_progess) {
- EMSG("call_begin(): transaction already in progress");
+ EMSG("transaction already in progress");
goto out;
}
- if (req_len > UINT32_MAX) {
- EMSG("call_begin(): req_len too big");
+ if (req_len > this_context->shared_mem_required_size) {
+ EMSG("req_len too big");
goto out;
}
this_context->is_call_transaction_in_progess = true;
handle = this_context;
- if (req_len > 0) {
- this_context->req_buf = shared_buffer;
- *req_buf = this_context->req_buf;
- this_context->req_len = req_len;
- } else {
- *req_buf = NULL;
- this_context->req_buf = NULL;
- this_context->req_len = req_len;
- }
+ *req_buf = (req_len > 0) ? this_context->req_buf : NULL;
+
+ this_context->req_buf = *req_buf;
+ this_context->req_len = req_len;
+
out:
return handle;
}
@@ -67,22 +63,26 @@
if (handle != this_context || opstatus == NULL ||
resp_buf == NULL || resp_len == NULL) {
- EMSG("call_invoke(): invalid arguments");
+ EMSG("invalid arguments");
status = TS_RPC_ERROR_INVALID_PARAMETER;
goto out;
}
if (!this_context->is_call_transaction_in_progess) {
- EMSG("call_invoke(): transaction was not started");
+ EMSG("transaction was not started");
status = TS_RPC_ERROR_NOT_READY;
goto out;
}
+ if (this_context->req_len > UINT32_MAX) {
+ EMSG("req_len is too large");
+ goto out;
+ }
+
req.destination_id = this_context->dest_partition_id;
req.source_id = own_id;
req.args[SP_CALL_ARGS_IFACE_ID_OPCODE] =
FFA_CALL_ARGS_COMBINE_IFACE_ID_OPCODE(this_context->dest_iface_id, opcode);
- //TODO: downcast problem?
req.args[SP_CALL_ARGS_REQ_DATA_LEN] = (uint32_t)this_context->req_len;
req.args[SP_CALL_ARGS_ENCODING] = this_context->rpc_caller.encoding;
@@ -93,7 +93,6 @@
req.args[SP_CALL_ARGS_CALLER_ID] = 0;
sp_res = sp_msg_send_direct_req(&req, &resp);
-
if (sp_res != SP_RESULT_OK) {
EMSG("sp_msg_send_direct_req(): error %"PRId32, sp_res);
goto out;
@@ -103,12 +102,16 @@
status = resp.args[SP_CALL_ARGS_RESP_RPC_STATUS];
*opstatus = (rpc_status_t)((int32_t)resp.args[SP_CALL_ARGS_RESP_OP_STATUS]);
- if (this_context->resp_len > 0) {
- this_context->resp_buf = shared_buffer;
- } else {
- this_context->resp_buf = NULL;
+ if (this_context->resp_len > this_context->shared_mem_required_size) {
+ EMSG("invalid response length");
+ goto out;
}
+ if (this_context->resp_len > 0)
+ this_context->resp_buf = shared_buffer;
+ else
+ this_context->resp_buf = NULL;
+
*resp_buf = this_context->resp_buf;
*resp_len = this_context->resp_len;
out:
@@ -120,7 +123,7 @@
struct ffarpc_caller *this_context = (struct ffarpc_caller *)context;
if (handle != this_context) {
- EMSG("call_end(): invalid arguments");
+ EMSG("invalid arguments");
return;
}
@@ -131,51 +134,51 @@
this_context->is_call_transaction_in_progess = false;
}
-struct rpc_caller *ffarpc_caller_init(struct ffarpc_caller *s)
+struct rpc_caller *ffarpc_caller_init(struct ffarpc_caller *caller)
{
- struct rpc_caller *base = &s->rpc_caller;
+ struct rpc_caller *base = &caller->rpc_caller;
- rpc_caller_init(base, s);
+ rpc_caller_init(base, caller);
base->call_begin = call_begin;
base->call_invoke = call_invoke;
base->call_end = call_end;
- s->dest_partition_id = 0;
- s->dest_iface_id = 0;
- s->shared_mem_handle = 0;
- s->shared_mem_required_size = sizeof(shared_buffer);
- s->req_buf = NULL;
- s->req_len = 0;
- s->resp_buf = NULL;
- s->resp_len = 0;
- s->is_call_transaction_in_progess = false;
+ caller->dest_partition_id = 0;
+ caller->dest_iface_id = 0;
+ caller->shared_mem_handle = 0;
+ caller->shared_mem_required_size = sizeof(shared_buffer);
+ caller->req_buf = NULL;
+ caller->req_len = 0;
+ caller->resp_buf = NULL;
+ caller->resp_len = 0;
+ caller->is_call_transaction_in_progess = false;
return base;
}
-void ffarpc_caller_deinit(struct ffarpc_caller *s)
+void ffarpc_caller_deinit(struct ffarpc_caller *caller)
{
- s->rpc_caller.context = NULL;
- s->rpc_caller.call_begin = NULL;
- s->rpc_caller.call_invoke = NULL;
- s->rpc_caller.call_end = NULL;
+ caller->rpc_caller.context = NULL;
+ caller->rpc_caller.call_begin = NULL;
+ caller->rpc_caller.call_invoke = NULL;
+ caller->rpc_caller.call_end = NULL;
}
uint32_t ffarpc_caller_discover(const uint8_t *uuid, uint16_t *sp_ids, uint32_t sp_max_cnt)
{
- ffa_result ffa_res;
- sp_result sp_res;
+ ffa_result ffa_res = FFA_OK;
+ sp_result sp_res = SP_RESULT_OK;
const void *rx_buf_addr = NULL;
size_t rx_buf_size = 0;
+ const struct ffa_partition_information *partitions = NULL;
uint32_t sp_cnt = 0;
- uint32_t i;
+ uint32_t i = 0;
if (uuid == NULL || sp_ids == NULL || sp_max_cnt == 0) {
- EMSG("ffarpc_caller_discover(): invalid arguments");
+ EMSG("invalid arguments");
goto out;
}
- //TODO: not sure if this cast is acceptable
ffa_res = ffa_partition_info_get((struct ffa_uuid *)uuid, &sp_cnt);
if (ffa_res != FFA_OK) {
EMSG("ffa_partition_info_get(): error %"PRId32, ffa_res);
@@ -188,12 +191,9 @@
goto out;
}
- const struct ffa_partition_information *partitions =
- (const struct ffa_partition_information *)rx_buf_addr;
-
- for (i = 0; i < sp_cnt && i < sp_max_cnt; i++) {
+ partitions = (const struct ffa_partition_information *)rx_buf_addr;
+ for (i = 0; i < sp_cnt && i < sp_max_cnt; i++)
sp_ids[i] = partitions[i].partition_id;
- }
ffa_res = ffa_rx_release();
if (ffa_res != FFA_OK) {
@@ -204,19 +204,24 @@
return sp_cnt;
}
-int ffarpc_caller_open(struct ffarpc_caller *s, uint16_t dest_partition_id, uint16_t dest_iface_id)
+int ffarpc_caller_open(struct ffarpc_caller *caller, uint16_t dest_partition_id,
+ uint16_t dest_iface_id)
{
- //TODO: revise return type, error handling
sp_result sp_res = SP_RESULT_OK;
struct sp_msg req = { 0 };
struct sp_msg resp = { 0 };
- struct sp_memory_descriptor desc = { };
- struct sp_memory_access_descriptor acc_desc = { };
- struct sp_memory_region region = { };
+ struct sp_memory_descriptor desc = { 0 };
+ struct sp_memory_access_descriptor acc_desc = { 0 };
+ struct sp_memory_region region = { 0 };
uint64_t handle = 0;
+ if (caller->shared_mem_required_size > UINT32_MAX) {
+ EMSG("memory is too large to share");
+ return -1;
+ }
+
desc.sender_id = own_id;
desc.memory_type = sp_memory_type_normal_memory;
desc.mem_region_attr.normal_memory.cacheability = sp_cacheability_write_back;
@@ -241,58 +246,53 @@
FFA_CALL_ARGS_COMBINE_IFACE_ID_OPCODE(FFA_CALL_MGMT_IFACE_ID, FFA_CALL_OPCODE_SHARE_BUF);
req.args[SP_CALL_ARGS_SHARE_MEM_HANDLE_LSW] = (uint32_t)(handle & UINT32_MAX);
req.args[SP_CALL_ARGS_SHARE_MEM_HANDLE_MSW] = (uint32_t)(handle >> 32);
- //TODO: downcast
- req.args[SP_CALL_ARGS_SHARE_MEM_SIZE] = (uint32_t)(s->shared_mem_required_size);
+ req.args[SP_CALL_ARGS_SHARE_MEM_SIZE] = (uint32_t)(caller->shared_mem_required_size);
sp_res = sp_msg_send_direct_req(&req, &resp);
-
if (sp_res != SP_RESULT_OK) {
EMSG("sp_msg_send_direct_req(): error %"PRId32, sp_res);
return -1;
}
- s->dest_partition_id = dest_partition_id;
- s->dest_iface_id = dest_iface_id;
- s->shared_mem_handle = handle;
+ caller->dest_partition_id = dest_partition_id;
+ caller->dest_iface_id = dest_iface_id;
+ caller->shared_mem_handle = handle;
return 0;
}
-int ffarpc_caller_close(struct ffarpc_caller *s)
+int ffarpc_caller_close(struct ffarpc_caller *caller)
{
- //TODO: revise return type, error handling
sp_result sp_res = SP_RESULT_OK;
struct sp_msg req = { 0 };
struct sp_msg resp = { 0 };
+ uint32_t handle_lo = 0, handle_hi = 0;
- uint32_t handle_lo, handle_hi;
-
- handle_lo = (uint32_t)(s->shared_mem_handle & UINT32_MAX);
- handle_hi = (uint32_t)(s->shared_mem_handle >> 32);
+ handle_lo = (uint32_t)(caller->shared_mem_handle & UINT32_MAX);
+ handle_hi = (uint32_t)(caller->shared_mem_handle >> 32);
req.source_id = own_id;
- req.destination_id = s->dest_partition_id;
+ req.destination_id = caller->dest_partition_id;
req.args[SP_CALL_ARGS_IFACE_ID_OPCODE] =
FFA_CALL_ARGS_COMBINE_IFACE_ID_OPCODE(FFA_CALL_MGMT_IFACE_ID, FFA_CALL_OPCODE_UNSHARE_BUF);
req.args[SP_CALL_ARGS_SHARE_MEM_HANDLE_LSW] = handle_lo;
req.args[SP_CALL_ARGS_SHARE_MEM_HANDLE_MSW] = handle_hi;
sp_res = sp_msg_send_direct_req(&req, &resp);
-
if (sp_res != SP_RESULT_OK) {
EMSG("sp_msg_send_direct_req(): error %"PRId32, sp_res);
return -1;
}
- sp_res = sp_memory_reclaim(s->shared_mem_handle, 0);
+ sp_res = sp_memory_reclaim(caller->shared_mem_handle, 0);
if (sp_res != SP_RESULT_OK) {
EMSG("sp_memory_reclaim(): error %"PRId32, sp_res);
return -1;
}
- s->dest_partition_id = 0;
- s->dest_iface_id = 0;
- s->shared_mem_handle = 0;
+ caller->dest_partition_id = 0;
+ caller->dest_iface_id = 0;
+ caller->shared_mem_handle = 0;
return 0;
}