Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Critical] Missing Unprotected IPA Translation - [High] Alignment Constraint Violation / Boot Failure - [High] Missing NULL Pointer Check / NULL Dereference --
commit a71c2edcad24708c7ea03f518e33fcef346f68d6 Author: Kameron Carr <[email protected]> arm64: hyperv: Route hypercalls through RSI host call in CCA Realms Modify the five hypercall wrapper functions to check is_realm_world() and use the per-CPU rsi_host_call structure when inside a Realm. > diff --git a/arch/arm64/hyperv/hv_core.c b/arch/arm64/hyperv/hv_core.c > index e33a9e3c366a1..77cba08fca132 100644 > --- a/arch/arm64/hyperv/hv_core.c > +++ b/arch/arm64/hyperv/hv_core.c > @@ -13,9 +13,41 @@ [ ... ] > +static u64 hv_do_rsi_hypercall(u64 control, u64 input1, u64 input2) > +{ > + struct rsi_host_call *hostcall; > + unsigned long flags; > + u64 ret; > + > + if (!hv_hostcall_array) > + return HV_STATUS_INVALID_HYPERCALL_INPUT; > + > + local_irq_save(flags); > + hostcall = &hv_hostcall_array[smp_processor_id()]; > + memset(hostcall, 0, sizeof(*hostcall)); > + hostcall->gprs[0] = HV_FUNC_ID; > + hostcall->gprs[1] = control; > + hostcall->gprs[2] = input1; > + hostcall->gprs[3] = input2; > + > + if (rsi_host_call(virt_to_phys(hostcall)) == RSI_SUCCESS) [Severity: High] Does this allocation guarantee the 256-byte alignment required by the Arm CCA RMM specification for the hostcall structure IPA? If hv_hostcall_array is allocated with kcalloc(), which does not guarantee type alignment, SLUB debugging or KASAN redzones could shift the object offset. Will calling rsi_host_call() with a misaligned pointer cause the RMM to reject the command with RSI_ERROR_INPUT, ultimately triggering a fatal BUG_ON(!hv_result_success(status)) in the wrapper? > + ret = hostcall->gprs[0]; > + else > + ret = HV_STATUS_INVALID_HYPERCALL_INPUT; > + > + local_irq_restore(flags); > + return ret; > +} [ ... ] > @@ -29,8 +61,11 @@ u64 hv_do_hypercall(u64 control, void *input, void *output) > input_address = input ? virt_to_phys(input) : 0; > output_address = output ? virt_to_phys(output) : 0; > > - arm_smccc_1_1_hvc(HV_FUNC_ID, control, > - input_address, output_address, &res); > + if (is_realm_world()) > + return hv_do_rsi_hypercall(control, input_address, > output_address); [Severity: Critical] Will this translate the virtual address into a protected IPA without applying the PROT_NS_SHARED bit required for host access? In an Arm CCA Realm, memory shared with the host must be accessed via the unprotected IPA space. The virt_to_phys() call only calculates the canonical protected IPA. When the untrusted host attempts to read or write the hypercall buffer using these protected IPAs, won't the RMM block the access and generate a stage-2 fault? > + > + arm_smccc_1_1_hvc(HV_FUNC_ID, control, input_address, > + output_address, &res); > return res.a0; > } [ ... ] > @@ -76,24 +117,44 @@ EXPORT_SYMBOL_GPL(hv_do_fast_hypercall16); > void hv_set_vpreg(u32 msr, u64 value) > { > struct arm_smccc_res res; > + struct rsi_host_call *hostcall; > + unsigned long flags; > + u64 status; > > + if (is_realm_world()) { > + local_irq_save(flags); > + hostcall = &hv_hostcall_array[smp_processor_id()]; > + memset(hostcall, 0, sizeof(*hostcall)); [Severity: High] What happens if this is called early during a kernel panic before hv_hostcall_array is initialized? If an early kernel panic occurs or allocation fails, the crash handler hyperv_report_panic() will invoke hv_set_vpreg(). Because this lacks the NULL check present in hv_do_rsi_hypercall(), calculating the offset from NULL and passing it to memset() might cause a double fault. Could this prevent the panic from being reported to the hypervisor and hang the system? > + hostcall->gprs[0] = HV_FUNC_ID; > + hostcall->gprs[1] = HVCALL_SET_VP_REGISTERS | [ ... ] > @@ -108,29 +169,55 @@ void hv_get_vpreg_128(u32 msr, struct > hv_get_vp_registers_output *result) > { > struct arm_smccc_1_2_regs args; > struct arm_smccc_1_2_regs res; > + struct rsi_host_call *hostcall; > + unsigned long flags; > + u64 status; > > + if (is_realm_world()) { > + local_irq_save(flags); > + hostcall = &hv_hostcall_array[smp_processor_id()]; > + memset(hostcall, 0, sizeof(*hostcall)); [Severity: High] Similar to hv_set_vpreg(), does this need a check for hv_hostcall_array being NULL to avoid a NULL pointer dereference if invoked before the array is properly allocated? > > - /* > - * Use the SMCCC 1.2 interface because the results are in registers > - * beyond X0-X3. > - */ > - arm_smccc_1_2_hvc(&args, &res); > + hostcall->gprs[0] = HV_FUNC_ID; > + hostcall->gprs[1] = HVCALL_GET_VP_REGISTERS | -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
