Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] arm64: hyperv: Missing NULL check on hv_hostcall_array causes NULL dereference - [High] arm64: hyperv: Shared per-CPU RSI hostcall buffer breaks NMI re-entrancy - [High] arm64: hyperv: kcalloc allocation fails to guarantee 256-byte RSI alignment requirement - [Medium] arm64: hyperv: Missing PROT_NS_SHARED bit on IPA passed to host in CCA Realms --
commit f459f1780582f30fc15313756e77abd8a5994839 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..0b5c89eb9df19 100644 > --- a/arch/arm64/hyperv/hv_core.c > +++ b/arch/arm64/hyperv/hv_core.c [ ... ] > @@ -12,11 +12,44 @@ > #include <linux/export.h> > #include <linux/mm.h> > #include <linux/arm-smccc.h> > +#include <linux/arm-rsi-cmds.h> > #include <linux/module.h> > +#include <linux/smp.h> > #include <asm-generic/bug.h> > #include <hyperv/hvhdk.h> > #include <asm/mshyperv.h> > > + > +/* > + * hv_do_rsi_hypercall - Helper function to invoke a hypercall from a > + * Realm world using the RSI interface. > + */ > +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); [Severity: High] Does local_irq_save() provide sufficient protection here? Since it disables normal IRQs but does not mask NMIs (or ARM64 pseudo-NMIs), could an NMI handler (such as a watchdog or kernel panic tracing) that makes a hypercall overwrite the active hostcall buffer and corrupt the outer hypercall's state? > + 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] Could the physical address passed to rsi_host_call() be unaligned here? The ARM CCA RSI ABI mandates that the SMC_RSI_HOST_CALL structure be 256-byte aligned. Since hv_hostcall_array is dynamically allocated via kcalloc() (which guarantees only standard architecture minimum alignment like 8 or 16 bytes), could this pass an unaligned block and cause the RMM to reject the host call? > + ret = hostcall->gprs[0]; > + else > + ret = HV_STATUS_INVALID_HYPERCALL_INPUT; > + > + local_irq_restore(flags); > + return ret; > +} > + > /* > * hv_do_hypercall- Invoke the specified hypercall > */ > @@ -29,8 +62,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: Medium] Does this pass a Protected IPA to the host without setting the PROT_NS_SHARED bit? The virt_to_phys() macro extracts the base physical address but does not include the shared memory bit. If the host relies on this address to access the payload buffer, will it fail to access the guest's Protected memory unless the Hyper-V host implicitly ORs the shared bit for all Realm hypercalls? [ ... ] > @@ -76,24 +118,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; > > - arm_smccc_1_1_hvc(HV_FUNC_ID, > - HVCALL_SET_VP_REGISTERS | HV_HYPERCALL_FAST_BIT | > - HV_HYPERCALL_REP_COMP_1, > - HV_PARTITION_ID_SELF, > - HV_VP_INDEX_SELF, > - msr, > - 0, > - value, > - 0, > - &res); > + if (is_realm_world()) { > + local_irq_save(flags); [Severity: High] As in hv_do_rsi_hypercall(), could an NMI interrupting this critical section corrupt the shared per-CPU rsi_host_call buffer? > + hostcall = &hv_hostcall_array[smp_processor_id()]; > + memset(hostcall, 0, sizeof(*hostcall)); [Severity: High] Could this result in a NULL pointer dereference if hyperv_init() has not been called or failed? The hv_do_rsi_hypercall() function explicitly checks for !hv_hostcall_array, but that protection appears to be missing here before calculating the offset and calling memset(). > + hostcall->gprs[0] = HV_FUNC_ID; > + hostcall->gprs[1] = HVCALL_SET_VP_REGISTERS | > + HV_HYPERCALL_FAST_BIT | > + HV_HYPERCALL_REP_COMP_1; > + hostcall->gprs[2] = HV_PARTITION_ID_SELF; [ ... ] > @@ -108,29 +170,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; > > - args.a0 = HV_FUNC_ID; > - args.a1 = HVCALL_GET_VP_REGISTERS | HV_HYPERCALL_FAST_BIT | > - HV_HYPERCALL_REP_COMP_1; > - args.a2 = HV_PARTITION_ID_SELF; > - args.a3 = HV_VP_INDEX_SELF; > - args.a4 = msr; > + 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() above, is a check for !hv_hostcall_array needed here to prevent a potential NULL pointer dereference on initialization error paths? > > - /* > - * 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 | > + HV_HYPERCALL_FAST_BIT | > + HV_HYPERCALL_REP_COMP_1; [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
