From 1f8752ea2f2f0cdc89bc709248f043f3d557a090 Mon Sep 17 00:00:00 2001 From: Daniel Skinstad Drabitzius Date: Mon, 25 Nov 2024 19:44:09 +0100 Subject: [PATCH 1/2] fix: double free in `mender_flash_abort_deployment` A double free can occur in `mender_flash_abort_deployment`, so call `FREE_AND_NULL` Changelog: None Ticket: None Signed-off-by: Daniel Skinstad Drabitzius --- core/src/mender-zephyr-image-update-module.c | 4 ++-- include/mender-flash.h | 4 ++-- platform/flash/generic/weak/src/mender-flash.c | 4 ++-- platform/flash/posix/src/mender-flash.c | 10 +++++----- platform/flash/zephyr/src/mender-flash.c | 10 +++++----- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/core/src/mender-zephyr-image-update-module.c b/core/src/mender-zephyr-image-update-module.c index 366d8686..fec609a2 100644 --- a/core/src/mender-zephyr-image-update-module.c +++ b/core/src/mender-zephyr-image-update-module.c @@ -141,7 +141,7 @@ mender_zephyr_image_set_pending_image(MENDER_NDEBUG_UNUSED mender_update_state_t return MENDER_FAIL; } - if (MENDER_OK != (ret = mender_flash_set_pending_image(mcu_boot_flash_handle))) { + if (MENDER_OK != (ret = mender_flash_set_pending_image(&mcu_boot_flash_handle))) { mender_log_error("Unable to set boot partition"); return ret; } @@ -153,7 +153,7 @@ mender_zephyr_image_abort_deployment(MENDER_NDEBUG_UNUSED mender_update_state_t assert(MENDER_UPDATE_STATE_FAILURE == state); mender_err_t ret; - if (MENDER_OK != (ret = mender_flash_abort_deployment(mcu_boot_flash_handle))) { + if (MENDER_OK != (ret = mender_flash_abort_deployment(&mcu_boot_flash_handle))) { mender_log_error("Unable to abort deployment"); return ret; } diff --git a/include/mender-flash.h b/include/mender-flash.h index 4cd43883..eee96c98 100644 --- a/include/mender-flash.h +++ b/include/mender-flash.h @@ -57,14 +57,14 @@ mender_err_t mender_flash_close(void *handle); * @param handle Handle from mender_flash_open * @return MENDER_OK if the function succeeds, error code otherwise */ -mender_err_t mender_flash_set_pending_image(void *handle); +mender_err_t mender_flash_set_pending_image(void **handle); /** * @brief Abort current deployment * @param handle Handle from mender_flash_open * @return MENDER_OK if the function succeeds, error code otherwise */ -mender_err_t mender_flash_abort_deployment(void *handle); +mender_err_t mender_flash_abort_deployment(void **handle); /** * @brief Mark image valid and cancel rollback if this is still pending diff --git a/platform/flash/generic/weak/src/mender-flash.c b/platform/flash/generic/weak/src/mender-flash.c index 663101df..d867def4 100755 --- a/platform/flash/generic/weak/src/mender-flash.c +++ b/platform/flash/generic/weak/src/mender-flash.c @@ -53,7 +53,7 @@ mender_flash_close(void *handle) { } __attribute__((weak)) mender_err_t -mender_flash_set_pending_image(void *handle) { +mender_flash_set_pending_image(void **handle) { (void)handle; @@ -62,7 +62,7 @@ mender_flash_set_pending_image(void *handle) { } __attribute__((weak)) mender_err_t -mender_flash_abort_deployment(void *handle) { +mender_flash_abort_deployment(void **handle) { (void)handle; diff --git a/platform/flash/posix/src/mender-flash.c b/platform/flash/posix/src/mender-flash.c index d1b9e243..8958dff4 100644 --- a/platform/flash/posix/src/mender-flash.c +++ b/platform/flash/posix/src/mender-flash.c @@ -101,13 +101,13 @@ mender_flash_close(void *handle) { } mender_err_t -mender_flash_set_pending_image(void *handle) { +mender_flash_set_pending_image(void **handle) { FILE *file; mender_err_t ret = MENDER_OK; /* Check flash handle */ - if (NULL != handle) { + if (NULL != *handle) { /* Write request update file */ if (NULL == (file = fopen(MENDER_FLASH_REQUEST_UPGRADE, "wb"))) { @@ -122,13 +122,13 @@ mender_flash_set_pending_image(void *handle) { } mender_err_t -mender_flash_abort_deployment(void *handle) { +mender_flash_abort_deployment(void **handle) { /* Check flash handle */ - if (NULL != handle) { + if (NULL != *handle) { /* Release memory */ - fclose(handle); + fclose(*handle); } return MENDER_OK; diff --git a/platform/flash/zephyr/src/mender-flash.c b/platform/flash/zephyr/src/mender-flash.c index 9a335e00..fdcefb2f 100644 --- a/platform/flash/zephyr/src/mender-flash.c +++ b/platform/flash/zephyr/src/mender-flash.c @@ -90,12 +90,12 @@ mender_flash_close(void *handle) { } mender_err_t -mender_flash_set_pending_image(void *handle) { +mender_flash_set_pending_image(void **handle) { int result; /* Check flash handle */ - if (NULL != handle) { + if (NULL != *handle) { /* Set new boot partition */ if (0 != (result = boot_request_upgrade(BOOT_UPGRADE_TEST))) { @@ -104,7 +104,7 @@ mender_flash_set_pending_image(void *handle) { } /* Release memory */ - free(handle); + FREE_AND_NULL(*handle); } else { /* This should not happen! */ @@ -116,10 +116,10 @@ mender_flash_set_pending_image(void *handle) { } mender_err_t -mender_flash_abort_deployment(void *handle) { +mender_flash_abort_deployment(void **handle) { /* Release memory */ - free(handle); + FREE_AND_NULL(*handle); return MENDER_OK; } From d37607b215a873d575a533dbe70b885f6e7c09fe Mon Sep 17 00:00:00 2001 From: Daniel Skinstad Drabitzius Date: Mon, 25 Nov 2024 19:44:11 +0100 Subject: [PATCH 2/2] fix: fail deployment when it's aborted Changelog: Title Ticket: MEN-7693 Signed-off-by: Daniel Skinstad Drabitzius --- core/src/mender-api.c | 4 +++ core/src/mender-client.c | 33 ++++++++++++++----- include/mender-utils.h | 1 + platform/tls/generic/mbedtls/src/mender-tls.c | 3 ++ 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/core/src/mender-api.c b/core/src/mender-api.c index 47dc4582..fce57f01 100644 --- a/core/src/mender-api.c +++ b/core/src/mender-api.c @@ -486,6 +486,10 @@ mender_api_publish_deployment_status(const char *id, mender_deployment_status_t if (204 == status) { /* No response expected */ ret = MENDER_OK; + } else if (409 == status) { + /* Deployment aborted */ + mender_api_print_response_error(response, status); + ret = MENDER_ABORTED; } else { mender_api_print_response_error(response, status); ret = MENDER_FAIL; diff --git a/core/src/mender-client.c b/core/src/mender-client.c index ce5e07f0..c9747736 100644 --- a/core/src/mender-client.c +++ b/core/src/mender-client.c @@ -857,6 +857,15 @@ set_and_store_state(const mender_update_state_t state) { return ret; } +/** + * @brief Publish deployment status and check if deployment is aborted + */ +#define PUBLISH_DEPLOYMENT_STATUS_CHECK_ABORTED(ret, aborted_deployment, id, deployment_status) \ + aborted_deployment = (MENDER_ABORTED == mender_client_publish_deployment_status(id, deployment_status)); \ + if (aborted_deployment) { \ + ret = MENDER_FAIL; \ + } + static mender_err_t mender_client_update_work_function(void) { mender_err_t ret = MENDER_OK; @@ -865,9 +874,10 @@ mender_client_update_work_function(void) { mender_artifact_ctx_t *mender_artifact_ctx = NULL; /* Check for deployment */ - mender_api_deployment_data_t *deployment = NULL; - mender_update_state_t update_state = MENDER_UPDATE_STATE_DOWNLOAD; - const char *deployment_id = NULL; + mender_api_deployment_data_t *deployment = NULL; + mender_update_state_t update_state = MENDER_UPDATE_STATE_DOWNLOAD; + const char *deployment_id = NULL; + bool aborted_deployment = false; /* reset the currently used update module */ mender_update_module = NULL; @@ -943,12 +953,13 @@ mender_client_update_work_function(void) { mender_log_info("Downloading artifact with id '%s', name '%s', uri '%s'", deployment->id, deployment->artifact_name, deployment->uri); } #endif - mender_client_publish_deployment_status(deployment->id, MENDER_DEPLOYMENT_STATUS_DOWNLOADING); - /* Set deployment_id */ deployment_id = deployment->id; - if (MENDER_OK == (ret = mender_download_artifact(deployment->uri, mender_client_deployment_data, &mender_update_module))) { + PUBLISH_DEPLOYMENT_STATUS_CHECK_ABORTED(ret, aborted_deployment, deployment->id, MENDER_DEPLOYMENT_STATUS_DOWNLOADING); + + if ((!aborted_deployment) + && (MENDER_OK == (ret = mender_download_artifact(deployment->uri, mender_client_deployment_data, &mender_update_module)))) { assert(NULL != mender_update_module); /* Get artifact context if artifact download succeeded */ @@ -990,8 +1001,8 @@ mender_client_update_work_function(void) { case MENDER_UPDATE_STATE_INSTALL: mender_log_info("Download done, installing artifact"); - mender_client_publish_deployment_status(deployment_id, MENDER_DEPLOYMENT_STATUS_INSTALLING); - if (NULL != mender_update_module->callbacks[update_state]) { + PUBLISH_DEPLOYMENT_STATUS_CHECK_ABORTED(ret, aborted_deployment, deployment_id, MENDER_DEPLOYMENT_STATUS_INSTALLING); + if ((MENDER_OK == ret) && NULL != mender_update_module->callbacks[update_state]) { ret = mender_update_module->callbacks[update_state](update_state, (mender_update_state_data_t)NULL); } if ((MENDER_OK == ret) && !mender_update_module->requires_reboot) { @@ -1007,7 +1018,7 @@ mender_client_update_work_function(void) { case MENDER_UPDATE_STATE_REBOOT: assert(mender_update_module->requires_reboot); mender_log_info("Artifact installation done, rebooting"); - mender_client_publish_deployment_status(deployment_id, MENDER_DEPLOYMENT_STATUS_REBOOTING); + PUBLISH_DEPLOYMENT_STATUS_CHECK_ABORTED(ret, aborted_deployment, deployment_id, MENDER_DEPLOYMENT_STATUS_REBOOTING); if ((MENDER_OK == ret) && (NULL != mender_update_module->callbacks[update_state])) { /* Save the next state before running the reboot callback -- * if there is an interrupt (power, crash,...) right after, @@ -1052,6 +1063,7 @@ mender_client_update_work_function(void) { ret = mender_update_module->callbacks[update_state](update_state, (mender_update_state_data_t)NULL); } if (MENDER_OK == ret) { + /* We don't check for MENDER_ABORTED, as it's too late if it has reached this far */ mender_client_publish_deployment_status(deployment_id, MENDER_DEPLOYMENT_STATUS_SUCCESS); } NEXT_STATE; @@ -1069,6 +1081,9 @@ mender_client_update_work_function(void) { if (!mender_update_module->supports_rollback) { mender_log_warning("Rollback not supported for artifacts of type '%s'", mender_update_module->artifact_type); ret = MENDER_FAIL; + } else if (aborted_deployment) { + /* Don't rollback if deployment is aborted */ + ret = MENDER_FAIL; } else if (NULL != mender_update_module->callbacks[update_state]) { ret = mender_update_module->callbacks[update_state](update_state, (mender_update_state_data_t)NULL); } diff --git a/include/mender-utils.h b/include/mender-utils.h index 59adc7f9..61d7ce5a 100644 --- a/include/mender-utils.h +++ b/include/mender-utils.h @@ -92,6 +92,7 @@ typedef enum { MENDER_NOT_FOUND = -2, /**< Not found */ MENDER_NOT_IMPLEMENTED = -3, /**< Not implemented */ MENDER_LOOP_DETECTED = -4, /**< Loop detected */ + MENDER_ABORTED = -5, /**< Aborted */ } mender_err_t; /** diff --git a/platform/tls/generic/mbedtls/src/mender-tls.c b/platform/tls/generic/mbedtls/src/mender-tls.c index 547d5621..24d8a343 100644 --- a/platform/tls/generic/mbedtls/src/mender-tls.c +++ b/platform/tls/generic/mbedtls/src/mender-tls.c @@ -195,6 +195,9 @@ mender_tls_init_authentication_keys(mender_err_t (*get_user_provided_keys)(char case MENDER_LOOP_DETECTED: assert(false && "Unexpected return value"); /* fallthrough */ + case MENDER_ABORTED: + assert(false && "Unexpected return value"); + /* fallthrough */ case MENDER_FAIL: mender_log_error("Unable to get authentication keys from store"); return MENDER_FAIL;