From 32f9a96c31cdedd8407e213795ffefd5260d285d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Ga=C5=82at?= Date: Wed, 6 Mar 2024 15:23:06 +0000 Subject: [PATCH] heap: initialize buffers to 0 before use and check for NULL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When memory from heap is used to store struct, it is needed to initialize it with zero to avoid random values on on elements not set explicitelly. KRKNWK-18597 Signed-off-by: Robert Gałat --- samples/sid_end_device/src/cli/app.c | 6 +++- samples/sid_end_device/src/cli/app_shell.c | 10 ++++++ samples/sid_end_device/src/file_transfer.c | 2 ++ samples/sid_end_device/src/hello/app.c | 27 +++++++++++++-- .../src/sensor_monitoring/app.c | 6 +++- .../src/sensor_monitoring/app_tx.c | 10 ++++++ samples/sid_end_device/src/sidewalk.c | 33 +++++++++---------- tests/unit_tests/sid_dut_shell/src/main.c | 1 + 8 files changed, 73 insertions(+), 22 deletions(-) diff --git a/samples/sid_end_device/src/cli/app.c b/samples/sid_end_device/src/cli/app.c index 3cb4c0924a..0dad5c53d7 100644 --- a/samples/sid_end_device/src/cli/app.c +++ b/samples/sid_end_device/src/cli/app.c @@ -63,7 +63,11 @@ static void on_sidewalk_status_changed(const struct sid_status *status, void *co int err = 0; uint32_t new_link_mask = status->detail.link_status_mask; struct sid_status *new_status = sid_hal_malloc(sizeof(struct sid_status)); - memcpy(new_status, status, sizeof(struct sid_status)); + if (!new_status) { + LOG_ERR("Failed to allocate memory for new status value"); + } else { + memcpy(new_status, status, sizeof(struct sid_status)); + } sidewalk_event_send(SID_EVENT_NEW_STATUS, new_status); switch (status->state) { diff --git a/samples/sid_end_device/src/cli/app_shell.c b/samples/sid_end_device/src/cli/app_shell.c index 7b858045f5..5cffbc14fb 100644 --- a/samples/sid_end_device/src/cli/app_shell.c +++ b/samples/sid_end_device/src/cli/app_shell.c @@ -214,6 +214,7 @@ static int cmd_sid_option(cli_event_t event, enum sid_option option, void *data, if (!p_opt) { return -ENOMEM; } + memset(p_opt, 0x0, sizeof(*p_opt)); p_opt->option = option; p_opt->data_len = len; if (data) { @@ -489,6 +490,10 @@ int cmd_sid_send(const struct shell *shell, int32_t argc, const char **argv) } sidewalk_msg_t *send = sid_hal_malloc(sizeof(sidewalk_msg_t)); + if (!send) { + return -ENOMEM; + } + memset(send, 0x0, sizeof(*send)); memcpy(&send->msg, &msg, sizeof(struct sid_msg)); memcpy(&send->desc, &desc, sizeof(struct sid_msg_desc)); @@ -875,6 +880,11 @@ int cmd_sid_option_gc(const struct shell *shell, int32_t argc, const char **argv return -EINVAL; } uint32_t *p_link_mask = sid_hal_malloc(sizeof(uint32_t)); + if (!p_link_mask) { + return -ENOMEM; + } + + memset(p_link_mask, 0x0, sizeof(*p_link_mask)); cli_parse_link_mask_opt((uint8_t)link_type, p_link_mask); int err = cmd_sid_option_get_input_data(SID_OPTION_GET_LINK_POLICY_AUTO_CONNECT_PARAMS, diff --git a/samples/sid_end_device/src/file_transfer.c b/samples/sid_end_device/src/file_transfer.c index 5c1b49ae19..c4b21784ab 100644 --- a/samples/sid_end_device/src/file_transfer.c +++ b/samples/sid_end_device/src/file_transfer.c @@ -57,6 +57,7 @@ static void on_transfer_request(const struct sid_bulk_data_transfer_request *con transfer_response->reject_reason = SID_BULK_DATA_TRANSFER_REJECT_REASON_NO_SPACE; return; } + memset(ptr, 0x0, sizeof(transfer_request->minimum_scratch_buffer_size)); // accept all requests if only we have avaliable memory for scratch buffer buffer_repo[repo_index].memory_slab_for_transfer = ptr; @@ -82,6 +83,7 @@ static void on_data_received(const struct sid_bulk_data_transfer_desc *const des LOG_ERR("Failed to allocate memory for received data descriptor"); return; } + memset(args, 0x0, sizeof(*args)); args->desc = *desc; args->buffer = (struct sid_bulk_data_transfer_buffer *)buffer; args->context = context; diff --git a/samples/sid_end_device/src/hello/app.c b/samples/sid_end_device/src/hello/app.c index 3c6a041254..8f87e71a38 100644 --- a/samples/sid_end_device/src/hello/app.c +++ b/samples/sid_end_device/src/hello/app.c @@ -50,9 +50,18 @@ static void on_sidewalk_msg_received(const struct sid_msg_desc *msg_desc, const if (msg_desc->type == SID_MSG_TYPE_GET || msg_desc->type == SID_MSG_TYPE_SET) { LOG_INF("Send echo message"); sidewalk_msg_t *echo = sid_hal_malloc(sizeof(sidewalk_msg_t)); - + if (!echo) { + LOG_ERR("Failed to allocate event context for echo message"); + return; + } + memset(echo, 0x0, sizeof(*echo)); echo->msg.size = msg->size; echo->msg.data = sid_hal_malloc(echo->msg.size); + if (!echo->msg.data) { + LOG_ERR("Failed to allocate memory for message echo data"); + sid_hal_free(echo); + return; + } memcpy(echo->msg.data, msg->data, echo->msg.size); echo->desc.type = (msg_desc->type == SID_MSG_TYPE_GET) ? SID_MSG_TYPE_RESPONSE : @@ -113,7 +122,11 @@ static void on_sidewalk_status_changed(const struct sid_status *status, void *co int err = 0; uint32_t new_link_mask = status->detail.link_status_mask; struct sid_status *new_status = sid_hal_malloc(sizeof(struct sid_status)); - memcpy(new_status, status, sizeof(struct sid_status)); + if (!new_status) { + LOG_ERR("Failed to allocate memory for new status value"); + } else { + memcpy(new_status, status, sizeof(struct sid_status)); + } sidewalk_event_send(SID_EVENT_NEW_STATUS, new_status); switch (status->state) { @@ -170,9 +183,19 @@ static void app_button_handler(uint32_t event) LOG_INF("Send hello message"); const char payload[] = "hello"; sidewalk_msg_t *hello = sid_hal_malloc(sizeof(sidewalk_msg_t)); + if (!hello) { + LOG_ERR("Failed to alloc memory for message context"); + return; + } + memset(hello, 0x0, sizeof(*hello)); hello->msg.size = sizeof(payload); hello->msg.data = sid_hal_malloc(hello->msg.size); + if (!hello->msg.data) { + sid_hal_free(hello); + LOG_ERR("Failed to allocate memory for message data"); + return; + } memcpy(hello->msg.data, payload, hello->msg.size); hello->desc.type = SID_MSG_TYPE_NOTIFY; diff --git a/samples/sid_end_device/src/sensor_monitoring/app.c b/samples/sid_end_device/src/sensor_monitoring/app.c index 5f2c0e21a7..5817426380 100644 --- a/samples/sid_end_device/src/sensor_monitoring/app.c +++ b/samples/sid_end_device/src/sensor_monitoring/app.c @@ -93,7 +93,11 @@ static void on_sidewalk_factory_reset(void *context) static void on_sidewalk_status_changed(const struct sid_status *status, void *context) { struct sid_status *new_status = sid_hal_malloc(sizeof(struct sid_status)); - memcpy(new_status, status, sizeof(struct sid_status)); + if (!new_status) { + LOG_ERR("Failed to allocate memory for new status value"); + } else { + memcpy(new_status, status, sizeof(struct sid_status)); + } sidewalk_event_send(SID_EVENT_NEW_STATUS, new_status); int err = 0; diff --git a/samples/sid_end_device/src/sensor_monitoring/app_tx.c b/samples/sid_end_device/src/sensor_monitoring/app_tx.c index d2faa4b75b..f1ee8f2147 100644 --- a/samples/sid_end_device/src/sensor_monitoring/app_tx.c +++ b/samples/sid_end_device/src/sensor_monitoring/app_tx.c @@ -95,8 +95,18 @@ static int app_tx_demo_msg_send(struct sid_parse_state *state, uint8_t *buffer, // Send sidewalk message sidewalk_msg_t *sid_msg = sid_hal_malloc(sizeof(sidewalk_msg_t)); + if (!sid_msg) { + LOG_ERR("Failed to alloc memory for message context"); + return -ENOMEM; + } + memset(sid_msg, 0x0, sizeof(*sid_msg)); sid_msg->msg.size = state->offset; sid_msg->msg.data = sid_hal_malloc(sid_msg->msg.size); + if (!sid_msg->msg.data) { + sid_hal_free(sid_msg); + LOG_ERR("Failed to allocate memory for message data"); + return -ENOMEM; + } memcpy(sid_msg->msg.data, msg_buffer, sid_msg->msg.size); memcpy(&sid_msg->desc, sid_desc, sizeof(struct sid_msg_desc)); diff --git a/samples/sid_end_device/src/sidewalk.c b/samples/sid_end_device/src/sidewalk.c index 141985a1ec..020f2cc7e1 100644 --- a/samples/sid_end_device/src/sidewalk.c +++ b/samples/sid_end_device/src/sidewalk.c @@ -162,13 +162,13 @@ static void state_sidewalk_run(void *o) sid_error_t e = SID_ERROR_NONE; switch (sm->event.id) { - case SID_EVENT_SIDEWALK: + case SID_EVENT_SIDEWALK: { e = sid_process(sm->sid->handle); if (e) { LOG_ERR("sid process err %d", (int)e); } - break; - case SID_EVENT_FACTORY_RESET: + } break; + case SID_EVENT_FACTORY_RESET: { #ifdef CONFIG_SID_END_DEVICE_PERSISTENT_LINK_MASK (void)settings_utils_link_mask_set(0); #endif /* CONFIG_SIDEWALK_PERSISTENT_LINK_MASK */ @@ -176,8 +176,8 @@ static void state_sidewalk_run(void *o) if (e) { LOG_ERR("sid factory reset err %d", (int)e); } - break; - case SID_EVENT_LINK_SWITCH: + } break; + case SID_EVENT_LINK_SWITCH: { static uint32_t new_link_mask = DEFAULT_LM; switch (sm->sid->config.link_mask) { @@ -250,8 +250,8 @@ static void state_sidewalk_run(void *o) } } #endif /* CONFIG_SID_END_DEVICE_AUTO_CONN_REQ */ - break; - case SID_EVENT_NORDIC_DFU: + } break; + case SID_EVENT_NORDIC_DFU: { #ifdef CONFIG_SIDEWALK_FILE_TRANSFER app_file_transfer_demo_deinit(sm->sid->handle); #endif @@ -260,8 +260,8 @@ static void state_sidewalk_run(void *o) LOG_ERR("sid deinit err %d", (int)e); } smf_set_state(SMF_CTX(sm), &sid_states[STATE_DFU]); - break; - case SID_EVENT_NEW_STATUS: + } break; + case SID_EVENT_NEW_STATUS: { struct sid_status *p_status = (struct sid_status *)sm->event.ctx; if (!p_status) { LOG_ERR("sid new status is NULL"); @@ -270,9 +270,8 @@ static void state_sidewalk_run(void *o) memcpy(&sm->sid->last_status, p_status, sizeof(struct sid_status)); sid_hal_free(p_status); - - break; - case SID_EVENT_SEND_MSG: + } break; + case SID_EVENT_SEND_MSG: { sidewalk_msg_t *p_msg = (sidewalk_msg_t *)sm->event.ctx; if (!p_msg) { LOG_ERR("sid send msg is NULL"); @@ -286,9 +285,8 @@ static void state_sidewalk_run(void *o) LOG_DBG("sid send (type: %d, id: %u)", (int)p_msg->desc.type, p_msg->desc.id); sid_hal_free(p_msg->msg.data); sid_hal_free(p_msg); - - break; - case SID_EVENT_CONNECT: + } break; + case SID_EVENT_CONNECT: { if (!(sm->sid->config.link_mask & SID_LINK_TYPE_1)) { LOG_ERR("Can not request connection - BLE not enabled"); return; @@ -297,7 +295,7 @@ static void state_sidewalk_run(void *o) if (e) { LOG_ERR("sid conn req err %d", (int)e); } - break; + } break; case SID_EVENT_FILE_TRANSFER: { #ifdef CONFIG_SIDEWALK_FILE_TRANSFER struct data_received_args *args = (struct data_received_args *)sm->event.ctx; @@ -366,10 +364,9 @@ static void state_dfu_entry(void *o) static void state_dfu_run(void *o) { sm_t *sm = (sm_t *)o; - + int err = -ENOTSUP; switch (sm->event.id) { case SID_EVENT_NORDIC_DFU: - int err = -ENOTSUP; #if defined(CONFIG_SIDEWALK_DFU_SERVICE_BLE) err = nordic_dfu_ble_stop(); #endif diff --git a/tests/unit_tests/sid_dut_shell/src/main.c b/tests/unit_tests/sid_dut_shell/src/main.c index 3f78550a32..b41dbfdfdb 100644 --- a/tests/unit_tests/sid_dut_shell/src/main.c +++ b/tests/unit_tests/sid_dut_shell/src/main.c @@ -1458,6 +1458,7 @@ void test_sid_set_option_ml(struct test_sid_set_option_params params) void test_sid_set_option_gc(struct test_sid_set_option_params params) { + sidewalk_parameters_set(params_sid_ok); int ret = cmd_sid_option_gc(NULL, params.argc, params.argv); verify_sid_option_asserts(params, ret);