From 8a6d0d262ae03db0a0bedd047a2df6f95e8823f6 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Thu, 27 Apr 2023 13:46:41 +0100 Subject: [PATCH] fix(psci): do not panic on illegal MPIDR Commit 66327414fb1e ("fix(psci): potential array overflow with cpu on") changed an assert in the PSCI library's psci_cpu_on_start() function to a runtime error message, followed by a panic. This does not seem right for two reasons: - We must not panic() triggered by conditions influenced by lower EL callers. If non-secure world provides illegal arguments to a PSCI call, we can easily detect this and return -PSCI_E_INVALID_PARAMS, as the PSCI spec demands. In fact this is done already, which brings us to the next reason: - psci_cpu_on_start() is effectively a function private to the PSCI library: its prototype is in psci_private.h. It's just not static because it lives in a different code file from the main PSCI code. We check for illegal MPID values already in psci_cpu_on(), and return an error value to the caller, as we should. This function is the ONLY caller of psci_cpu_on_start(), so there is no way we get an illegal target_cpu argument into this function. An assert() is thus the proper way to check for this. Mostly revert the patch mentioned above, just extending the assert so that it does also check for not exceeding the array boundaries. To harden the code, add a check against PLATFORM_MAX_CORE_COUNT in psci_validate_mpidr(), and return with the proper PSCI error code if this number is exceeded. This also fixes the sun50i_a64 build with DEBUG=1, which exceeded an SRAM limit due to the error message. Change-Id: I48fc58d96b0173da5b934750f4cadf7884ef5e42 Signed-off-by: Andre Przywara --- lib/psci/psci_common.c | 5 ++++- lib/psci/psci_on.c | 7 ++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/psci/psci_common.c b/lib/psci/psci_common.c index ebeb10be9..c89347659 100644 --- a/lib/psci/psci_common.c +++ b/lib/psci/psci_common.c @@ -829,8 +829,11 @@ void psci_release_pwr_domain_locks(unsigned int end_pwrlvl, ******************************************************************************/ int psci_validate_mpidr(u_register_t mpidr) { - if (plat_core_pos_by_mpidr(mpidr) < 0) + int pos = plat_core_pos_by_mpidr(mpidr); + + if ((pos < 0) || ((unsigned int)pos >= PLATFORM_CORE_COUNT)) { return PSCI_E_INVALID_PARAMS; + } return PSCI_E_SUCCESS; } diff --git a/lib/psci/psci_on.c b/lib/psci/psci_on.c index 6c6b23c5a..76eb50ce5 100644 --- a/lib/psci/psci_on.c +++ b/lib/psci/psci_on.c @@ -65,13 +65,10 @@ int psci_cpu_on_start(u_register_t target_cpu, unsigned int target_idx; /* Calling function must supply valid input arguments */ + assert(ret >= 0); + assert((unsigned int)ret < PLATFORM_CORE_COUNT); assert(ep != NULL); - if ((ret < 0) || (ret >= (int)PLATFORM_CORE_COUNT)) { - ERROR("Unexpected core index.\n"); - panic(); - } - target_idx = (unsigned int)ret; /* -- 2.39.5