diff options
| author | Stefan Kerkmann <[email protected]> | 2022-08-06 10:46:59 +0200 | 
|---|---|---|
| committer | GitHub <[email protected]> | 2022-08-06 18:46:59 +1000 | 
| commit | ed9bdcbc3608819e17ff7a11221e651bf51ec1cc (patch) | |
| tree | 36e87885a316d67edc7fe64a04ca0db90c890e4c | |
| parent | cac704241404908794514b7a534e58c96aff4d6b (diff) | |
| download | qmk_firmware-ed9bdcbc3608819e17ff7a11221e651bf51ec1cc.tar.gz qmk_firmware-ed9bdcbc3608819e17ff7a11221e651bf51ec1cc.zip | |
[Core] guard RPC invocation by checking RPC info against crc checksum (#17840)
| -rw-r--r-- | quantum/split_common/transactions.c | 18 | ||||
| -rw-r--r-- | quantum/split_common/transport.h | 9 | 
2 files changed, 18 insertions, 9 deletions
| diff --git a/quantum/split_common/transactions.c b/quantum/split_common/transactions.c index 9e3df534e3..719068908f 100644 --- a/quantum/split_common/transactions.c +++ b/quantum/split_common/transactions.c @@ -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)          [PUT_RPC_INFO]  = trans_initiator2target_initializer_cb(rpc_info, slave_rpc_info_callback),      [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),  #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;      // 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      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.      // 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[GET_RPC_RESP_DATA].target2initiator_buffer_size = split_shmem->rpc_info.s2m_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.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) {      // 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. -    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) {          split_transaction_desc_t *trans = &split_transaction_table[transaction_id];          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);          }      }  } diff --git a/quantum/split_common/transport.h b/quantum/split_common/transport.h index e62679990a..06778ad14a 100644 --- a/quantum/split_common/transport.h +++ b/quantum/split_common/transport.h @@ -116,9 +116,12 @@ typedef struct _split_slave_pointing_sync_t {  #if defined(SPLIT_TRANSACTION_IDS_KB) || defined(SPLIT_TRANSACTION_IDS_USER)  typedef struct _rpc_sync_info_t { -    int8_t  transaction_id; -    uint8_t m2s_length; -    uint8_t s2m_length; +    uint8_t checksum; +    struct { +        int8_t  transaction_id; +        uint8_t m2s_length; +        uint8_t s2m_length; +    } payload;  } rpc_sync_info_t;  #endif // defined(SPLIT_TRANSACTION_IDS_KB) || defined(SPLIT_TRANSACTION_IDS_USER) | 
