From ed1ad62351ac17b8cfb50134c1bceb8e5d203651 Mon Sep 17 00:00:00 2001 From: Andrey Tolstoy Date: Thu, 4 Nov 2021 03:53:21 +0700 Subject: [PATCH 1/3] [gen3] hal: zero out invalid user modules --- hal/src/nRF52840/ota_flash_hal.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/hal/src/nRF52840/ota_flash_hal.cpp b/hal/src/nRF52840/ota_flash_hal.cpp index e37ce76d50..24bf65bc69 100644 --- a/hal/src/nRF52840/ota_flash_hal.cpp +++ b/hal/src/nRF52840/ota_flash_hal.cpp @@ -188,8 +188,17 @@ int HAL_System_Info(hal_system_info_t* info, bool construct, void* reserved) } bool valid = fetch_module(module, bounds, false, MODULE_VALIDATION_INTEGRITY); valid = valid && (module->validity_checked == module->validity_result); - if (valid && bounds->module_function == MODULE_FUNCTION_USER_PART) { - user_module_found = true; + if (bounds->module_function == MODULE_FUNCTION_USER_PART) { + if (valid) { + user_module_found = true; + } else { + // IMPORTANT: we should not be reporting invalid user module in the describe + // as it may contain garbage data and may be presented as for example + // system module with invalid dependency on some non-existent module + // FIXME: we should potentially do a similar thing for other module types + // but for now only tackling user modules + memset(module, 0, sizeof(*module)); + } } } } From fe8b2477c3256cc9ad7a5493c0cd98c15537cbc1 Mon Sep 17 00:00:00 2001 From: Andrey Tolstoy Date: Thu, 4 Nov 2021 05:25:07 +0700 Subject: [PATCH 2/3] [test] wiring/gen3_invalid_compat_user_app --- .../wiring/gen3_invalid_compat_user_app | 1 + .../application.cpp | 40 +++++++++++ .../gen3_invalid_compat_user_app.spec.js | 3 + .../gen3_invalid_compat_user_app/test.cpp | 69 +++++++++++++++++++ 4 files changed, 113 insertions(+) create mode 120000 user/tests/integration/wiring/gen3_invalid_compat_user_app create mode 100644 user/tests/wiring/gen3_invalid_compat_user_app/application.cpp create mode 100644 user/tests/wiring/gen3_invalid_compat_user_app/gen3_invalid_compat_user_app.spec.js create mode 100644 user/tests/wiring/gen3_invalid_compat_user_app/test.cpp diff --git a/user/tests/integration/wiring/gen3_invalid_compat_user_app b/user/tests/integration/wiring/gen3_invalid_compat_user_app new file mode 120000 index 0000000000..31a342d00e --- /dev/null +++ b/user/tests/integration/wiring/gen3_invalid_compat_user_app @@ -0,0 +1 @@ +../../wiring/gen3_invalid_compat_user_app \ No newline at end of file diff --git a/user/tests/wiring/gen3_invalid_compat_user_app/application.cpp b/user/tests/wiring/gen3_invalid_compat_user_app/application.cpp new file mode 100644 index 0000000000..649f4a0b81 --- /dev/null +++ b/user/tests/wiring/gen3_invalid_compat_user_app/application.cpp @@ -0,0 +1,40 @@ +/* + * Copyright (c) 2021 Particle Industries, Inc. All rights reserved. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation, either + * version 3 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see . + */ + +#ifndef PARTICLE_TEST_RUNNER + +#include "application.h" +#include "unit-test/unit-test.h" + +SYSTEM_MODE(SEMI_AUTOMATIC); + +// make clean all TEST=wiring/no_fixture_long_running PLATFORM=electron -s COMPILE_LTO=n program-dfu DEBUG_BUILD=y +// make clean all TEST=wiring/no_fixture_long_running PLATFORM=electron -s COMPILE_LTO=n program-dfu DEBUG_BUILD=y USE_THREADING=y +// +// Serial1LogHandler logHandler(115200, LOG_LEVEL_ALL, { +// { "comm", LOG_LEVEL_NONE }, // filter out comm messages +// { "system", LOG_LEVEL_INFO } // only info level for system messages +// }); + +UNIT_TEST_APP(); + +// Enable threading if compiled with "USE_THREADING=y" +#if PLATFORM_THREADING == 1 && USE_THREADING == 1 +SYSTEM_THREAD(ENABLED); +#endif + +#endif // PARTICLE_TEST_RUNNER \ No newline at end of file diff --git a/user/tests/wiring/gen3_invalid_compat_user_app/gen3_invalid_compat_user_app.spec.js b/user/tests/wiring/gen3_invalid_compat_user_app/gen3_invalid_compat_user_app.spec.js new file mode 100644 index 0000000000..f151a6e595 --- /dev/null +++ b/user/tests/wiring/gen3_invalid_compat_user_app/gen3_invalid_compat_user_app.spec.js @@ -0,0 +1,3 @@ +suite('Gen 3 invalid compat user app'); + +platform('gen3'); diff --git a/user/tests/wiring/gen3_invalid_compat_user_app/test.cpp b/user/tests/wiring/gen3_invalid_compat_user_app/test.cpp new file mode 100644 index 0000000000..cc21ed7e76 --- /dev/null +++ b/user/tests/wiring/gen3_invalid_compat_user_app/test.cpp @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2021 Particle Industries, Inc. All rights reserved. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation, either + * version 3 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see . + */ + +#define PARTICLE_USE_UNSTABLE_API +#include "application.h" +#include "unit-test/unit-test.h" + +#if HAL_PLATFORM_GEN != 3 +#error "Unsupported platform" +#endif // HAL_PLATFORM_GEN != 3 + +struct padded_module_info { + uint8_t padding[0x4000]; + module_info_t info; +}; + +// This structure is placed at exactly where 128KB compat +// user module would be located due to 64KB alignment and padding +__attribute__((aligned(64 * 1024), used)) const padded_module_info brokenModule = { + .padding = {}, + .info = { + .module_start_address = (void*)0x123, + .module_end_address = (void*)0x1234, + .reserved = 0, + .flags = 0, + .module_version = 12345, + .platform_id = 100, + .module_function = 123, + .module_index = 123, + .dependency = { + .module_function = MODULE_FUNCTION_RESOURCE, + .module_index = 11, + .module_version = 11 + }, + .dependency2 = { + .module_function = MODULE_FUNCTION_RESOURCE, + .module_index = 12, + .module_version = 12 + } + } +}; + +test(00_system_describe_does_not_contain_invalid_compat_user_app) { + hal_system_info_t info = {}; + info.size = sizeof(info); + system_info_get_unstable(&info, 0, nullptr); + SCOPE_GUARD({ + system_info_free_unstable(&info, nullptr); + }); + + assertFalse(info.modules == nullptr); + for (unsigned i = 0; i < info.module_count; i++) { + assertFalse(!memcmp(&info.modules[i].info, &brokenModule.info, sizeof(module_info_t))); + } +} From 498cbb1a344ca79a9fb66d0b803ba71d14bec26a Mon Sep 17 00:00:00 2001 From: Andrey Tolstoy Date: Thu, 4 Nov 2021 08:50:59 +0700 Subject: [PATCH 3/3] address review comments --- hal/src/nRF52840/ota_flash_hal.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hal/src/nRF52840/ota_flash_hal.cpp b/hal/src/nRF52840/ota_flash_hal.cpp index 24bf65bc69..5cd9c79035 100644 --- a/hal/src/nRF52840/ota_flash_hal.cpp +++ b/hal/src/nRF52840/ota_flash_hal.cpp @@ -188,7 +188,7 @@ int HAL_System_Info(hal_system_info_t* info, bool construct, void* reserved) } bool valid = fetch_module(module, bounds, false, MODULE_VALIDATION_INTEGRITY); valid = valid && (module->validity_checked == module->validity_result); - if (bounds->module_function == MODULE_FUNCTION_USER_PART) { + if (bounds->store == MODULE_STORE_MAIN && bounds->module_function == MODULE_FUNCTION_USER_PART) { if (valid) { user_module_found = true; } else {