From e60c18471fc7488cc0bf1dc7eae3b43be77045a4 Mon Sep 17 00:00:00 2001 From: Manish Pandey Date: Fri, 27 Oct 2023 11:45:44 +0100 Subject: [PATCH] fix(smccc): ensure that mpidr passed through SMC is valid There are various SMC calls which pass mpidr as an argument which is currently tested at random places in SMC call path. To make the mpidr validation check consistent across SMC calls, do this check as part of SMC argument validation. This patch introduce a helper function is_valid_mpidr() to validate mpidr and call it as part of validating SMC arguments at starting of SMC handlers (which expect mpidr as an argument). Signed-off-by: Manish Pandey Change-Id: I11ea50e22caf17896cf4b2059b87029b2ba136b1 --- include/plat/common/platform.h | 14 ++++++++++++++ lib/pmf/pmf_main.c | 2 +- lib/pmf/pmf_smc.c | 4 ++++ lib/psci/psci_common.c | 14 -------------- lib/psci/psci_main.c | 26 +++++++++++++------------- lib/psci/psci_on.c | 10 +--------- lib/psci/psci_private.h | 1 - lib/psci/psci_stat.c | 14 +++++++++++--- services/std_svc/sdei/sdei_main.c | 6 ++---- 9 files changed, 46 insertions(+), 45 deletions(-) diff --git a/include/plat/common/platform.h b/include/plat/common/platform.h index c92121f60..4d1b1c17c 100644 --- a/include/plat/common/platform.h +++ b/include/plat/common/platform.h @@ -80,6 +80,20 @@ unsigned int plat_my_core_pos(void); int plat_core_pos_by_mpidr(u_register_t mpidr); int plat_get_mbedtls_heap(void **heap_addr, size_t *heap_size); +/******************************************************************************* + * Simple routine to determine whether a mpidr is valid or not. + ******************************************************************************/ +static inline bool is_valid_mpidr(u_register_t mpidr) +{ + int pos = plat_core_pos_by_mpidr(mpidr); + + if ((pos < 0) || ((unsigned int)pos >= PLATFORM_CORE_COUNT)) { + return false; + } + + return true; +} + #if STACK_PROTECTOR_ENABLED /* * Return a new value to be used for the stack protection's canary. diff --git a/lib/pmf/pmf_main.c b/lib/pmf/pmf_main.c index bf0ad8388..b33f49c82 100644 --- a/lib/pmf/pmf_main.c +++ b/lib/pmf/pmf_main.c @@ -165,7 +165,7 @@ int pmf_get_timestamp_smc(unsigned int tid, /* Search for registered service. */ svc_desc = get_service(tid); - if ((svc_desc == NULL) || (plat_core_pos_by_mpidr(mpidr) < 0)) { + if (svc_desc == NULL) { *ts_value = 0; return -EINVAL; } else { diff --git a/lib/pmf/pmf_smc.c b/lib/pmf/pmf_smc.c index 71486dfcb..f3dd11276 100644 --- a/lib/pmf/pmf_smc.c +++ b/lib/pmf/pmf_smc.c @@ -26,6 +26,10 @@ uintptr_t pmf_smc_handler(unsigned int smc_fid, int rc; unsigned long long ts_value; + /* Determine if the cpu exists of not */ + if (!is_valid_mpidr(x2)) + return PSCI_E_INVALID_PARAMS; + if (((smc_fid >> FUNCID_CC_SHIFT) & FUNCID_CC_MASK) == SMC_32) { x1 = (uint32_t)x1; diff --git a/lib/psci/psci_common.c b/lib/psci/psci_common.c index 70bf77e2d..f9de432b2 100644 --- a/lib/psci/psci_common.c +++ b/lib/psci/psci_common.c @@ -817,20 +817,6 @@ void psci_release_pwr_domain_locks(unsigned int end_pwrlvl, } } -/******************************************************************************* - * Simple routine to determine whether a mpidr is valid or not. - ******************************************************************************/ -int psci_validate_mpidr(u_register_t mpidr) -{ - 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; -} - /******************************************************************************* * This function determines the full entrypoint information for the requested * PSCI entrypoint on power on/resume and returns it. diff --git a/lib/psci/psci_main.c b/lib/psci/psci_main.c index 326f125b6..a01553128 100644 --- a/lib/psci/psci_main.c +++ b/lib/psci/psci_main.c @@ -29,9 +29,8 @@ int psci_cpu_on(u_register_t target_cpu, int rc; entry_point_info_t ep; - /* Determine if the cpu exists of not */ - rc = psci_validate_mpidr(target_cpu); - if (rc != PSCI_E_SUCCESS) + /* Validate the target CPU */ + if (!is_valid_mpidr(target_cpu)) return PSCI_E_INVALID_PARAMS; /* Validate the entry point and get the entry_point_info */ @@ -245,19 +244,18 @@ int psci_cpu_off(void) int psci_affinity_info(u_register_t target_affinity, unsigned int lowest_affinity_level) { - int ret; unsigned int target_idx; + /* Validate the target affinity */ + if (!is_valid_mpidr(target_affinity)) + return PSCI_E_INVALID_PARAMS; + /* We dont support level higher than PSCI_CPU_PWR_LVL */ if (lowest_affinity_level > PSCI_CPU_PWR_LVL) return PSCI_E_INVALID_PARAMS; /* Calculate the cpu index of the target */ - ret = plat_core_pos_by_mpidr(target_affinity); - if (ret == -1) { - return PSCI_E_INVALID_PARAMS; - } - target_idx = (unsigned int)ret; + target_idx = (unsigned int) plat_core_pos_by_mpidr(target_affinity); /* * Generic management: @@ -285,6 +283,10 @@ int psci_migrate(u_register_t target_cpu) int rc; u_register_t resident_cpu_mpidr; + /* Validate the target cpu */ + if (!is_valid_mpidr(target_cpu)) + return PSCI_E_INVALID_PARAMS; + rc = psci_spd_migrate_info(&resident_cpu_mpidr); if (rc != PSCI_TOS_UP_MIG_CAP) return (rc == PSCI_TOS_NOT_UP_MIG_CAP) ? @@ -298,8 +300,7 @@ int psci_migrate(u_register_t target_cpu) return PSCI_E_NOT_PRESENT; /* Check the validity of the specified target cpu */ - rc = psci_validate_mpidr(target_cpu); - if (rc != PSCI_E_SUCCESS) + if (!is_valid_mpidr(target_cpu)) return PSCI_E_INVALID_PARAMS; assert((psci_spd_pm != NULL) && (psci_spd_pm->svc_migrate != NULL)); @@ -339,8 +340,7 @@ int psci_node_hw_state(u_register_t target_cpu, int rc; /* Validate target_cpu */ - rc = psci_validate_mpidr(target_cpu); - if (rc != PSCI_E_SUCCESS) + if (!is_valid_mpidr(target_cpu)) return PSCI_E_INVALID_PARAMS; /* Validate power_level against PLAT_MAX_PWR_LVL */ diff --git a/lib/psci/psci_on.c b/lib/psci/psci_on.c index 31875ff3d..b2797749f 100644 --- a/lib/psci/psci_on.c +++ b/lib/psci/psci_on.c @@ -61,15 +61,7 @@ int psci_cpu_on_start(u_register_t target_cpu, { int rc; aff_info_state_t target_aff_state; - int ret = plat_core_pos_by_mpidr(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); - - target_idx = (unsigned int)ret; + unsigned int target_idx = (unsigned int)plat_core_pos_by_mpidr(target_cpu); /* * This function must only be called on platforms where the diff --git a/lib/psci/psci_private.h b/lib/psci/psci_private.h index 04f93bd08..2eb4a9b70 100644 --- a/lib/psci/psci_private.h +++ b/lib/psci/psci_private.h @@ -286,7 +286,6 @@ extern const spd_pm_ops_t *psci_spd_pm; int psci_validate_power_state(unsigned int power_state, psci_power_state_t *state_info); void psci_query_sys_suspend_pwrstate(psci_power_state_t *state_info); -int psci_validate_mpidr(u_register_t mpidr); void psci_init_req_local_pwr_states(void); #if PSCI_OS_INIT_MODE void psci_update_req_local_pwr_states(unsigned int end_pwrlvl, diff --git a/lib/psci/psci_stat.c b/lib/psci/psci_stat.c index ad88d0708..bedb816df 100644 --- a/lib/psci/psci_stat.c +++ b/lib/psci/psci_stat.c @@ -181,10 +181,8 @@ static int psci_get_stat(u_register_t target_cpu, unsigned int power_state, psci_power_state_t state_info = { {PSCI_LOCAL_STATE_RUN} }; plat_local_state_t local_state; - /* Validate the target_cpu parameter and determine the cpu index */ + /* Determine the cpu index */ target_idx = (unsigned int) plat_core_pos_by_mpidr(target_cpu); - if (target_idx == (unsigned int) -1) - return PSCI_E_INVALID_PARAMS; /* Validate the power_state parameter */ if (psci_plat_pm_ops->translate_power_state_by_mpidr == NULL) @@ -228,6 +226,11 @@ u_register_t psci_stat_residency(u_register_t target_cpu, unsigned int power_state) { psci_stat_t psci_stat; + + /* Validate the target cpu */ + if (!is_valid_mpidr(target_cpu)) + return 0; + int rc = psci_get_stat(target_cpu, power_state, &psci_stat); if (rc == PSCI_E_SUCCESS) @@ -241,6 +244,11 @@ u_register_t psci_stat_count(u_register_t target_cpu, unsigned int power_state) { psci_stat_t psci_stat; + + /* Validate the target cpu */ + if (!is_valid_mpidr(target_cpu)) + return 0; + int rc = psci_get_stat(target_cpu, power_state, &psci_stat); if (rc == PSCI_E_SUCCESS) diff --git a/services/std_svc/sdei/sdei_main.c b/services/std_svc/sdei/sdei_main.c index 44178eddd..f421831fa 100644 --- a/services/std_svc/sdei/sdei_main.c +++ b/services/std_svc/sdei/sdei_main.c @@ -35,8 +35,6 @@ #define LOWEST_INTR_PRIORITY 0xff -#define is_valid_affinity(_mpidr) (plat_core_pos_by_mpidr(_mpidr) >= 0) - CASSERT(PLAT_SDEI_CRITICAL_PRI < PLAT_SDEI_NORMAL_PRI, sdei_critical_must_have_higher_priority); @@ -262,7 +260,7 @@ static int validate_flags(uint64_t flags, uint64_t mpidr) /* Validate flags */ switch (flags) { case SDEI_REGF_RM_PE: - if (!is_valid_affinity(mpidr)) + if (!is_valid_mpidr(mpidr)) return SDEI_EINVAL; break; case SDEI_REGF_RM_ANY: @@ -926,7 +924,7 @@ static int sdei_signal(int ev_num, uint64_t target_pe) return SDEI_EINVAL; /* Validate target */ - if (plat_core_pos_by_mpidr(target_pe) < 0) + if (!is_valid_mpidr(target_pe)) return SDEI_EINVAL; /* Raise SGI. Platform will validate target_pe */