From 471654264073e5bf645f86e0b522f7bd503473ef Mon Sep 17 00:00:00 2001 From: AlexeiFedorov Date: Wed, 13 Sep 2023 11:47:57 +0100 Subject: [PATCH 1/3] fix(rmm): fix static_checks warnings and errors Fixes static_checks warning and errors listed below: - docs/about/change-log.rst:52: WARNING: Possible repeated word: 'the' - lib/attestation/include/attestation_token.h:116: WARNING: please, no space before tabs - lib/attestation/include/attestation_token.h:118: WARNING: please, no space before tabs - lib/attestation/src/attestation_rnd.c:58: WARNING: please, no spaces at the start of a line - lib/attestation/src/attestation_rnd.c:59: WARNING: please, no spaces at the start of a line - lib/measurement/src/measurement.c:18: WARNING: Missing a blank line after declarations - lib/realm/src/buffer.c:231: WARNING: externs should be avoided in .c and .cpp files - lib/xlat/src/xlat_tables_core.c:128: WARNING: else is not generally useful after a break or return - plat/host/host_test/src/test_helpers.c:22: WARNING: externs should be avoided in .c and .cpp files - runtime/rmi/run.c:222: WARNING: break is not useful after a goto - runtime/rmi/run.c:228: WARNING: break is not useful after a goto - runtime/rsi/realm_attest.c:309: WARNING: Missing a blank line after declarations -plat/fvp/src/include/fvp_dram.h: ERROR: includes not in order. Include order should be rmm_el3_ifc.h, stddef.h, stdint.h Signed-off-by: AlexeiFedorov Change-Id: I46a7ad1216eae826b5adb9487b397f2ed783d932 --- docs/about/change-log.rst | 2 +- lib/attestation/include/attestation_token.h | 4 ++-- lib/attestation/src/attestation_rnd.c | 4 ++-- lib/measurement/src/measurement.c | 1 + lib/realm/src/buffer.c | 3 --- lib/realm/src/include/buffer_private.h | 2 ++ lib/xlat/src/xlat_tables_core.c | 14 ++++++-------- plat/fvp/src/include/fvp_dram.h | 2 +- plat/host/host_test/src/test_helpers.c | 4 ---- plat/host/host_test/src/test_private.h | 4 ++++ plat/host/host_test/src/utest_exit.cpp | 1 - runtime/rmi/run.c | 2 -- runtime/rsi/psci.c | 1 + 13 files changed, 20 insertions(+), 24 deletions(-) diff --git a/docs/about/change-log.rst b/docs/about/change-log.rst index 3a9d94da..d29e4516 100644 --- a/docs/about/change-log.rst +++ b/docs/about/change-log.rst @@ -49,7 +49,7 @@ Build/Testing improvements attestation initialization flow. Also a sample minimal Realm create, run and destroy sequence is added to showcase the RMI calls involved. -- Further improvements to the the unit test framework : +- Further improvements to the unit test framework : * Restore the sysreg state between test runs so each test gets a known sysreg state. diff --git a/lib/attestation/include/attestation_token.h b/lib/attestation/include/attestation_token.h index ff9fee92..807d0b5a 100644 --- a/lib/attestation/include/attestation_token.h +++ b/lib/attestation/include/attestation_token.h @@ -113,9 +113,9 @@ attest_realm_token_sign(struct attest_token_encode_ctx *me, * Combine realm token and platform token to top-level cca token * * attest_token_buf Pointer to the buffer where the token will be - * written. + * written. * attest_token_buf_size Size of the buffer where the token will be - * written. + * written. * realm_token_buf Pointer to the realm token. * realm_token_len Length of the realm token. * diff --git a/lib/attestation/src/attestation_rnd.c b/lib/attestation/src/attestation_rnd.c index ac87b718..190773b2 100644 --- a/lib/attestation/src/attestation_rnd.c +++ b/lib/attestation/src/attestation_rnd.c @@ -55,8 +55,8 @@ static int get_random_seed(unsigned char *output, size_t len) * For details see `MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG` in mbedtls/mbedtls_config.h */ psa_status_t mbedtls_psa_external_get_random( - mbedtls_psa_external_random_context_t *context, - uint8_t *output, size_t output_size, size_t *output_length) + mbedtls_psa_external_random_context_t *context, + uint8_t *output, size_t output_size, size_t *output_length) { int ret; unsigned int cpu_id = my_cpuid(); diff --git a/lib/measurement/src/measurement.c b/lib/measurement/src/measurement.c index a18769d1..568e18c5 100644 --- a/lib/measurement/src/measurement.c +++ b/lib/measurement/src/measurement.c @@ -15,6 +15,7 @@ static void measurement_print(unsigned char *measurement, const enum hash_algo algorithm) { unsigned int size = 0U; + assert(measurement != NULL); VERBOSE("Measurement "); diff --git a/lib/realm/src/buffer.c b/lib/realm/src/buffer.c index 0422027a..5f86fdde 100644 --- a/lib/realm/src/buffer.c +++ b/lib/realm/src/buffer.c @@ -227,9 +227,6 @@ void buffer_unmap(void *buf) buffer_arch_unmap(buf); } -bool memcpy_ns_read(void *dest, const void *ns_src, size_t size); -bool memcpy_ns_write(void *ns_dest, const void *src, size_t size); - /* * Map a Non secure granule @g into the slot @slot and read data from * this granule to @dest. Unmap the granule once the read is done. diff --git a/lib/realm/src/include/buffer_private.h b/lib/realm/src/include/buffer_private.h index 77b1a2a2..cf0bb1c1 100644 --- a/lib/realm/src/include/buffer_private.h +++ b/lib/realm/src/include/buffer_private.h @@ -21,3 +21,5 @@ struct xlat_llt_info *get_cached_llt_info(void); uintptr_t slot_to_va(enum buffer_slot slot); +bool memcpy_ns_read(void *dest, const void *ns_src, size_t size); +bool memcpy_ns_write(void *ns_dest, const void *src, size_t size); diff --git a/lib/xlat/src/xlat_tables_core.c b/lib/xlat/src/xlat_tables_core.c index bd3a5d36..c282cfe7 100644 --- a/lib/xlat/src/xlat_tables_core.c +++ b/lib/xlat/src/xlat_tables_core.c @@ -125,15 +125,13 @@ static action_t xlat_tables_map_region_action(const struct xlat_mmap_region *mm, * overwrite. */ return ACTION_NONE; - } else { - if (desc_type != INVALID_DESC) { - ERROR("%s (%u): Expected invalid descriptor\n", - __func__, __LINE__); - panic(); - } - return ACTION_WRITE_BLOCK_ENTRY; } - + if (desc_type != INVALID_DESC) { + ERROR("%s (%u): Expected invalid descriptor\n", + __func__, __LINE__); + panic(); + } + return ACTION_WRITE_BLOCK_ENTRY; } else { /* diff --git a/plat/fvp/src/include/fvp_dram.h b/plat/fvp/src/include/fvp_dram.h index 1bc0917f..054e6f11 100644 --- a/plat/fvp/src/include/fvp_dram.h +++ b/plat/fvp/src/include/fvp_dram.h @@ -6,9 +6,9 @@ #ifndef FVP_DRAM_H #define FVP_DRAM_H +#include #include #include -#include /* Maximum number of DRAM banks supported */ #define MAX_DRAM_NUM_BANKS 2UL diff --git a/plat/host/host_test/src/test_helpers.c b/plat/host/host_test/src/test_helpers.c index f31abd86..6a5eebca 100644 --- a/plat/host/host_test/src/test_helpers.c +++ b/plat/host/host_test/src/test_helpers.c @@ -17,10 +17,6 @@ #include #include -/* Implemented in init.c and needed here */ -void rmm_warmboot_main(void); -void rmm_main(void); - /* * Define and set the Boot Interface arguments. */ diff --git a/plat/host/host_test/src/test_private.h b/plat/host/host_test/src/test_private.h index 580950b0..192765fe 100644 --- a/plat/host/host_test/src/test_private.h +++ b/plat/host/host_test/src/test_private.h @@ -13,4 +13,8 @@ */ uintptr_t get_cb(enum cb_ids id); +/* Implemented in init.c and needed in test_helpers.c */ +void rmm_warmboot_main(void); +void rmm_main(void); + #endif /* TEST_PRIVATE_H */ diff --git a/plat/host/host_test/src/utest_exit.cpp b/plat/host/host_test/src/utest_exit.cpp index 8f936a59..abb8e73f 100644 --- a/plat/host/host_test/src/utest_exit.cpp +++ b/plat/host/host_test/src/utest_exit.cpp @@ -16,5 +16,4 @@ extern "C" { { TEST_EXIT; } - } diff --git a/runtime/rmi/run.c b/runtime/rmi/run.c index effc39b7..c6944356 100644 --- a/runtime/rmi/run.c +++ b/runtime/rmi/run.c @@ -215,13 +215,11 @@ unsigned long smc_rec_enter(unsigned long rec_addr, case REALM_STATE_NEW: ret = pack_return_code(RMI_ERROR_REALM, 0U); goto out_unmap_buffers; - break; case REALM_STATE_ACTIVE: break; case REALM_STATE_SYSTEM_OFF: ret = pack_return_code(RMI_ERROR_REALM, 1U); goto out_unmap_buffers; - break; default: assert(false); break; diff --git a/runtime/rsi/psci.c b/runtime/rsi/psci.c index 22b9283d..a3752e5c 100644 --- a/runtime/rsi/psci.c +++ b/runtime/rsi/psci.c @@ -111,6 +111,7 @@ static unsigned long rd_map_read_rec_count(struct granule *g_rd) { unsigned long rec_count; struct rd *rd = granule_map(g_rd, SLOT_RD); + assert(rd != NULL); rec_count = get_rd_rec_count_unlocked(rd); From 7a2f5884f59d5d625a69d828d10eaf9c9f19144e Mon Sep 17 00:00:00 2001 From: AlexeiFedorov Date: Thu, 14 Sep 2023 11:41:32 +0100 Subject: [PATCH 2/3] fix(rmm): add 'level_bound' failure condition Adds 'level_bound' failure condition for RMI_RTT_MAP_UNPROTECTED and RMI_RTT_UNMAP_UNPROTECTED command handlers. Signed-off-by: AlexeiFedorov Change-Id: Iee5cbea57adaf431ca56b83323bb776311287e5d --- runtime/rmi/rtt.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/runtime/rmi/rtt.c b/runtime/rmi/rtt.c index cd0ae481..d55db047 100644 --- a/runtime/rmi/rtt.c +++ b/runtime/rmi/rtt.c @@ -709,6 +709,10 @@ unsigned long smc_rtt_map_unprotected(unsigned long rd_addr, long level = (long)ulevel; struct smc_result res; + if ((level < RTT_MIN_BLOCK_LEVEL) || (level > RTT_PAGE_LEVEL)) { + return RMI_ERROR_INPUT; + } + if (!host_ns_s2tte_is_valid(s2tte, level)) { return RMI_ERROR_INPUT; } @@ -722,7 +726,14 @@ void smc_rtt_unmap_unprotected(unsigned long rd_addr, unsigned long ulevel, struct smc_result *res) { - return map_unmap_ns(rd_addr, map_addr, (long)ulevel, 0UL, UNMAP_NS, res); + long level = (long)ulevel; + + if ((level < RTT_MIN_BLOCK_LEVEL) || (level > RTT_PAGE_LEVEL)) { + res->x[0] = RMI_ERROR_INPUT; + return; + } + + map_unmap_ns(rd_addr, map_addr, level, 0UL, UNMAP_NS, res); } void smc_rtt_read_entry(unsigned long rd_addr, From 868a651173a43216c1bd149a95a17d5c8a0a7c25 Mon Sep 17 00:00:00 2001 From: AlexeiFedorov Date: Thu, 14 Sep 2023 13:21:11 +0100 Subject: [PATCH 3/3] fix(rmm): add 'ipa_bound' check Adds 'ipa_bound' failure condition in RMI_DATA_DESTROY command handler. Signed-off-by: AlexeiFedorov Change-Id: Idb78cba3349f02eac30b275ebfdae6a06394268e --- runtime/rmi/rtt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/runtime/rmi/rtt.c b/runtime/rmi/rtt.c index d55db047..c7a416d6 100644 --- a/runtime/rmi/rtt.c +++ b/runtime/rmi/rtt.c @@ -1022,7 +1022,8 @@ void smc_data_destroy(unsigned long rd_addr, rd = granule_map(g_rd, SLOT_RD); assert(rd != NULL); - if (!validate_map_addr(map_addr, RTT_PAGE_LEVEL, rd)) { + if (!addr_in_par(rd, map_addr) || + !validate_map_addr(map_addr, RTT_PAGE_LEVEL, rd)) { buffer_unmap(rd); granule_unlock(g_rd); res->x[0] = RMI_ERROR_INPUT;