diff --git a/subsys/mgmt/osdp/src/osdp_cp.c b/subsys/mgmt/osdp/src/osdp_cp.c index 3c8cfc2e8cc..e21747821de 100644 --- a/subsys/mgmt/osdp/src/osdp_cp.c +++ b/subsys/mgmt/osdp/src/osdp_cp.c @@ -80,6 +80,12 @@ int osdp_extract_address(int *address) return (pd_offset == CONFIG_OSDP_NUM_CONNECTED_PD) ? 0 : -1; } +static inline void assert_buf_len(int need, int have) +{ + __ASSERT(need < have, "OOM at build command: need:%d have:%d", + need, have); +} + static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len) { struct osdp_cmd *cmd = NULL; @@ -97,45 +103,50 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len) switch (pd->cmd_id) { case CMD_POLL: - __fallthrough; + assert_buf_len(CMD_POLL_LEN, max_len); + buf[len++] = pd->cmd_id; + ret = 0; + break; case CMD_LSTAT: - __fallthrough; + assert_buf_len(CMD_LSTAT_LEN, max_len); + buf[len++] = pd->cmd_id; + ret = 0; + break; case CMD_ISTAT: - __fallthrough; + assert_buf_len(CMD_ISTAT_LEN, max_len); + buf[len++] = pd->cmd_id; + ret = 0; + break; case CMD_OSTAT: - __fallthrough; + assert_buf_len(CMD_OSTAT_LEN, max_len); + buf[len++] = pd->cmd_id; + ret = 0; + break; case CMD_RSTAT: + assert_buf_len(CMD_RSTAT_LEN, max_len); buf[len++] = pd->cmd_id; ret = 0; break; case CMD_ID: - if (max_len < CMD_ID_LEN) { - break; - } + assert_buf_len(CMD_ID_LEN, max_len); buf[len++] = pd->cmd_id; buf[len++] = 0x00; ret = 0; break; case CMD_CAP: - if (max_len < CMD_CAP_LEN) { - break; - } + assert_buf_len(CMD_CAP_LEN, max_len); buf[len++] = pd->cmd_id; buf[len++] = 0x00; ret = 0; break; case CMD_DIAG: - if (max_len < CMD_DIAG_LEN) { - break; - } + assert_buf_len(CMD_DIAG_LEN, max_len); buf[len++] = pd->cmd_id; buf[len++] = 0x00; ret = 0; break; case CMD_OUT: - if (max_len < CMD_OUT_LEN) { - break; - } + assert_buf_len(CMD_OUT_LEN, max_len); cmd = (struct osdp_cmd *)pd->cmd_data; buf[len++] = pd->cmd_id; buf[len++] = cmd->output.output_no; @@ -145,9 +156,7 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len) ret = 0; break; case CMD_LED: - if (max_len < CMD_LED_LEN) { - break; - } + assert_buf_len(CMD_LED_LEN, max_len); cmd = (struct osdp_cmd *)pd->cmd_data; buf[len++] = pd->cmd_id; buf[len++] = cmd->led.reader; @@ -169,9 +178,7 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len) ret = 0; break; case CMD_BUZ: - if (max_len < CMD_BUZ_LEN) { - break; - } + assert_buf_len(CMD_BUZ_LEN, max_len); cmd = (struct osdp_cmd *)pd->cmd_data; buf[len++] = pd->cmd_id; buf[len++] = cmd->buzzer.reader; @@ -183,9 +190,7 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len) break; case CMD_TEXT: cmd = (struct osdp_cmd *)pd->cmd_data; - if (max_len < (CMD_TEXT_LEN + cmd->text.length)) { - break; - } + assert_buf_len(CMD_TEXT_LEN + cmd->text.length, max_len); buf[len++] = pd->cmd_id; buf[len++] = cmd->text.reader; buf[len++] = cmd->text.control_code; @@ -199,9 +204,7 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len) ret = 0; break; case CMD_COMSET: - if (max_len < CMD_COMSET_LEN) { - break; - } + assert_buf_len(CMD_COMSET_LEN, max_len); cmd = (struct osdp_cmd *)pd->cmd_data; buf[len++] = pd->cmd_id; buf[len++] = cmd->comset.address; @@ -217,9 +220,7 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len) LOG_ERR("Cannot perform KEYSET without SC!"); return OSDP_CP_ERR_GENERIC; } - if (max_len < CMD_KEYSET_LEN) { - break; - } + assert_buf_len(CMD_KEYSET_LEN, max_len); buf[len++] = pd->cmd_id; buf[len++] = 1; /* key type (1: SCBK) */ buf[len++] = 16; /* key length in bytes */ @@ -228,7 +229,8 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len) ret = 0; break; case CMD_CHLNG: - if (smb == NULL || max_len < CMD_CHLNG_LEN) { + assert_buf_len(CMD_CHLNG_LEN, max_len); + if (smb == NULL) { break; } osdp_fill_random(pd->sc.cp_random, 8); @@ -242,7 +244,8 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len) ret = 0; break; case CMD_SCRYPT: - if (smb == NULL || max_len < CMD_SCRYPT_LEN) { + assert_buf_len(CMD_SCRYPT_LEN, max_len); + if (smb == NULL) { break; } osdp_compute_cp_cryptogram(pd); @@ -489,6 +492,11 @@ static int cp_decode_response(struct osdp_pd *pd, uint8_t *buf, int len) return OSDP_CP_ERR_UNKNOWN; } + if (ret != OSDP_CP_ERR_NONE) { + LOG_ERR("Failed to decode response: REPLY(%02x) for CMD(%02x)", + pd->cmd_id, pd->reply_id); + } + if (pd->cmd_id != CMD_POLL) { LOG_DBG("CMD(%02x) REPLY(%02x)", pd->cmd_id, pd->reply_id); } diff --git a/subsys/mgmt/osdp/src/osdp_pd.c b/subsys/mgmt/osdp/src/osdp_pd.c index 228cf525f4c..f28f345ea19 100644 --- a/subsys/mgmt/osdp/src/osdp_pd.c +++ b/subsys/mgmt/osdp/src/osdp_pd.c @@ -299,7 +299,6 @@ static int pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len) #ifdef CONFIG_OSDP_SC_ENABLED case CMD_KEYSET: if (len != CMD_KEYSET_DATA_LEN) { - LOG_ERR("CMD_KEYSET length mismatch! %d/18", len); break; } /** @@ -342,7 +341,6 @@ static int pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len) break; } if (len != CMD_CHLNG_DATA_LEN) { - LOG_ERR("CMD_CHLNG length mismatch! %d/8", len); break; } osdp_sc_init(pd); @@ -356,7 +354,6 @@ static int pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len) } case CMD_SCRYPT: if (len != CMD_SCRYPT_DATA_LEN) { - LOG_ERR("CMD_SCRYPT length mismatch! %d/16", len); break; } for (i = 0; i < 16; i++) { @@ -373,11 +370,12 @@ static int pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len) return OSDP_PD_ERR_REPLY; } - if (ret != 0) { - LOG_ERR("Invalid command structure. CMD: %02x, Len: %d", - pd->cmd_id, len); + if (ret == OSDP_PD_ERR_GENERIC) { + LOG_ERR("Failed to decode command: CMD(%02x) Len:%d ret:%d", + pd->cmd_id, len, ret); pd->reply_id = REPLY_NAK; pd->cmd_data[0] = OSDP_PD_NAK_CMD_LEN; + ret = OSDP_PD_ERR_REPLY; } if (pd->cmd_id != CMD_POLL) { @@ -387,6 +385,12 @@ static int pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len) return ret; } +static inline void assert_buf_len(int need, int have) +{ + __ASSERT(need < have, "OOM at build command: need:%d have:%d", + need, have); +} + /** * Returns: * +ve: length of command @@ -404,25 +408,15 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len) #endif buf += data_off; max_len -= data_off; - if (max_len <= 0) { - LOG_ERR("Out of buffer space!"); - return -1; - } switch (pd->reply_id) { case REPLY_ACK: - if (max_len < REPLY_ACK_LEN) { - LOG_ERR("Out of buffer space!"); - break; - } + assert_buf_len(REPLY_ACK_LEN, max_len); buf[len++] = pd->reply_id; ret = OSDP_PD_ERR_NONE; break; case REPLY_PDID: - if (max_len < REPLY_PDID_LEN) { - LOG_ERR("Out of buffer space!"); - break; - } + assert_buf_len(REPLY_PDID_LEN, max_len); buf[len++] = pd->reply_id; buf[len++] = BYTE_0(pd->id.vendor_code); @@ -443,10 +437,7 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len) ret = OSDP_PD_ERR_NONE; break; case REPLY_PDCAP: - if (max_len < REPLY_PDCAP_LEN) { - LOG_ERR("Out of buffer space!"); - break; - } + assert_buf_len(REPLY_PDCAP_LEN, max_len); buf[len++] = pd->reply_id; for (i = 0; i < OSDP_PD_CAP_SENTINEL; i++) { if (pd->cap[i].function_code != i) { @@ -464,29 +455,20 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len) ret = OSDP_PD_ERR_NONE; break; case REPLY_LSTATR: - if (max_len < REPLY_LSTATR_LEN) { - LOG_ERR("Out of buffer space!"); - break; - } + assert_buf_len(REPLY_LSTATR_LEN, max_len); buf[len++] = pd->reply_id; buf[len++] = ISSET_FLAG(pd, PD_FLAG_TAMPER); buf[len++] = ISSET_FLAG(pd, PD_FLAG_POWER); ret = OSDP_PD_ERR_NONE; break; case REPLY_RSTATR: - if (max_len < REPLY_RSTATR_LEN) { - LOG_ERR("Out of buffer space!"); - break; - } + assert_buf_len(REPLY_RSTATR_LEN, max_len); buf[len++] = pd->reply_id; buf[len++] = ISSET_FLAG(pd, PD_FLAG_R_TAMPER); ret = OSDP_PD_ERR_NONE; break; case REPLY_COM: - if (max_len < REPLY_COM_LEN) { - LOG_ERR("Out of buffer space!"); - break; - } + assert_buf_len(REPLY_COM_LEN, max_len); /** * If COMSET succeeds, the PD must reply with the old params and * then switch to the new params from then then on. We have the @@ -517,10 +499,7 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len) ret = OSDP_PD_ERR_NONE; break; case REPLY_NAK: - if (max_len < REPLY_NAK_LEN) { - LOG_ERR("Fatal: insufficient space for sending NAK"); - return -1; - } + assert_buf_len(REPLY_NAK_LEN, max_len); buf[len++] = pd->reply_id; buf[len++] = pd->cmd_data[0]; ret = OSDP_PD_ERR_NONE; @@ -530,10 +509,7 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len) if (smb == NULL) { break; } - if (max_len < REPLY_CCRYPT_LEN) { - LOG_ERR("Out of buffer space!"); - return -1; - } + assert_buf_len(REPLY_CCRYPT_LEN, max_len); osdp_fill_random(pd->sc.pd_random, 8); osdp_compute_session_keys(pd); osdp_compute_pd_cryptogram(pd); @@ -556,10 +532,7 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len) if (smb == NULL) { break; } - if (max_len < REPLY_RMAC_I_LEN) { - LOG_ERR("Out of buffer space!"); - return -1; - } + assert_buf_len(REPLY_RMAC_I_LEN, max_len); osdp_compute_rmac_i(pd); buf[len++] = pd->reply_id; for (i = 0; i < 16; i++) { @@ -595,10 +568,7 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len) /* catch all errors and report it as a RECORD error to CP */ LOG_ERR("Failed to build REPLY(%02x); Sending NAK instead!", pd->reply_id); - if (max_len < REPLY_NAK_LEN) { - LOG_ERR("Fatal: insufficient space for sending NAK"); - return -1; - } + assert_buf_len(REPLY_NAK_LEN, max_len); buf[0] = REPLY_NAK; buf[1] = OSDP_PD_NAK_RECORD; len = 2;