[Core] guard RPC invocation by checking RPC info against crc checksum (#17840)

This commit is contained in:
Stefan Kerkmann 2022-08-06 10:46:59 +02:00 committed by GitHub
parent cac7042414
commit ed9bdcbc36
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 18 additions and 9 deletions

View File

@ -694,7 +694,7 @@ split_transaction_desc_t split_transaction_table[NUM_TOTAL_TRANSACTIONS] = {
#if defined(SPLIT_TRANSACTION_IDS_KB) || defined(SPLIT_TRANSACTION_IDS_USER) #if defined(SPLIT_TRANSACTION_IDS_KB) || defined(SPLIT_TRANSACTION_IDS_USER)
[PUT_RPC_INFO] = trans_initiator2target_initializer_cb(rpc_info, slave_rpc_info_callback), [PUT_RPC_INFO] = trans_initiator2target_initializer_cb(rpc_info, slave_rpc_info_callback),
[PUT_RPC_REQ_DATA] = trans_initiator2target_initializer(rpc_m2s_buffer), [PUT_RPC_REQ_DATA] = trans_initiator2target_initializer(rpc_m2s_buffer),
[EXECUTE_RPC] = trans_initiator2target_initializer_cb(rpc_info.transaction_id, slave_rpc_exec_callback), [EXECUTE_RPC] = trans_initiator2target_initializer_cb(rpc_info.payload.transaction_id, slave_rpc_exec_callback),
[GET_RPC_RESP_DATA] = trans_target2initiator_initializer(rpc_s2m_buffer), [GET_RPC_RESP_DATA] = trans_target2initiator_initializer(rpc_s2m_buffer),
#endif // defined(SPLIT_TRANSACTION_IDS_KB) || defined(SPLIT_TRANSACTION_IDS_USER) #endif // defined(SPLIT_TRANSACTION_IDS_KB) || defined(SPLIT_TRANSACTION_IDS_USER)
}; };
@ -760,7 +760,8 @@ bool transaction_rpc_exec(int8_t transaction_id, uint8_t initiator2target_buffer
if (target2initiator_buffer_size > RPC_S2M_BUFFER_SIZE) return false; if (target2initiator_buffer_size > RPC_S2M_BUFFER_SIZE) return false;
// Prepare the metadata block // Prepare the metadata block
rpc_sync_info_t info = {.transaction_id = transaction_id, .m2s_length = initiator2target_buffer_size, .s2m_length = target2initiator_buffer_size}; rpc_sync_info_t info = {.payload = {.transaction_id = transaction_id, .m2s_length = initiator2target_buffer_size, .s2m_length = target2initiator_buffer_size}};
info.checksum = crc8(&info.payload, sizeof(info.payload));
// Make sure the local side knows that we're not sending the full block of data // Make sure the local side knows that we're not sending the full block of data
split_transaction_table[PUT_RPC_REQ_DATA].initiator2target_buffer_size = initiator2target_buffer_size; split_transaction_table[PUT_RPC_REQ_DATA].initiator2target_buffer_size = initiator2target_buffer_size;
@ -791,18 +792,23 @@ void slave_rpc_info_callback(uint8_t initiator2target_buffer_size, const void *i
// Ignore the args -- the `split_shmem` already has the info, we just need to act upon it. // Ignore the args -- the `split_shmem` already has the info, we just need to act upon it.
// We must keep the `split_transaction_table` non-const, so that it is able to be modified at runtime. // We must keep the `split_transaction_table` non-const, so that it is able to be modified at runtime.
split_transaction_table[PUT_RPC_REQ_DATA].initiator2target_buffer_size = split_shmem->rpc_info.m2s_length; split_transaction_table[PUT_RPC_REQ_DATA].initiator2target_buffer_size = split_shmem->rpc_info.payload.m2s_length;
split_transaction_table[GET_RPC_RESP_DATA].target2initiator_buffer_size = split_shmem->rpc_info.s2m_length; split_transaction_table[GET_RPC_RESP_DATA].target2initiator_buffer_size = split_shmem->rpc_info.payload.s2m_length;
} }
void slave_rpc_exec_callback(uint8_t initiator2target_buffer_size, const void *initiator2target_buffer, uint8_t target2initiator_buffer_size, void *target2initiator_buffer) { void slave_rpc_exec_callback(uint8_t initiator2target_buffer_size, const void *initiator2target_buffer, uint8_t target2initiator_buffer_size, void *target2initiator_buffer) {
// We can assume that the buffer lengths are correctly set, now, given that sequentially the rpc_info callback was already executed. // We can assume that the buffer lengths are correctly set, now, given that sequentially the rpc_info callback was already executed.
// Go through the rpc_info and execute _that_ transaction's callback, with the scratch buffers as inputs. // Go through the rpc_info and execute _that_ transaction's callback, with the scratch buffers as inputs.
int8_t transaction_id = split_shmem->rpc_info.transaction_id; // As a safety precaution we check that the received payload matches its checksum first.
if (crc8(&split_shmem->rpc_info.payload, sizeof(split_shmem->rpc_info.payload)) != split_shmem->rpc_info.checksum) {
return;
}
int8_t transaction_id = split_shmem->rpc_info.payload.transaction_id;
if (transaction_id < NUM_TOTAL_TRANSACTIONS) { if (transaction_id < NUM_TOTAL_TRANSACTIONS) {
split_transaction_desc_t *trans = &split_transaction_table[transaction_id]; split_transaction_desc_t *trans = &split_transaction_table[transaction_id];
if (trans->slave_callback) { if (trans->slave_callback) {
trans->slave_callback(split_shmem->rpc_info.m2s_length, split_shmem->rpc_m2s_buffer, split_shmem->rpc_info.s2m_length, split_shmem->rpc_s2m_buffer); trans->slave_callback(split_shmem->rpc_info.payload.m2s_length, split_shmem->rpc_m2s_buffer, split_shmem->rpc_info.payload.s2m_length, split_shmem->rpc_s2m_buffer);
} }
} }
} }

View File

@ -116,9 +116,12 @@ typedef struct _split_slave_pointing_sync_t {
#if defined(SPLIT_TRANSACTION_IDS_KB) || defined(SPLIT_TRANSACTION_IDS_USER) #if defined(SPLIT_TRANSACTION_IDS_KB) || defined(SPLIT_TRANSACTION_IDS_USER)
typedef struct _rpc_sync_info_t { typedef struct _rpc_sync_info_t {
int8_t transaction_id; uint8_t checksum;
uint8_t m2s_length; struct {
uint8_t s2m_length; int8_t transaction_id;
uint8_t m2s_length;
uint8_t s2m_length;
} payload;
} rpc_sync_info_t; } rpc_sync_info_t;
#endif // defined(SPLIT_TRANSACTION_IDS_KB) || defined(SPLIT_TRANSACTION_IDS_USER) #endif // defined(SPLIT_TRANSACTION_IDS_KB) || defined(SPLIT_TRANSACTION_IDS_USER)