Re: [PATCH v2 5/6] arm64: hyperv: Route hypercalls through RSI host call in CCA Realms

[sashiko-bot@kernel.org]

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 <kameroncarr@linux.microsoft.com>

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/20260625173500.1995481-1-kameroncarr@linux.microsoft.com?part=5