Re: [PATCH v2 3/6] arm64: hyperv: Add per-CPU RSI host call infrastructure for CCA Realms

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The `hv_hostcall_array` is allocated with `kcalloc()`, which does not guarantee the strict 256-byte alignment required by the RMM for `SMC_RSI_HOST_CALL`.
- [High] If `hyperv_init()` fails, `hv_hostcall_array` is freed but panic notifiers are left registered, causing a fatal NULL pointer dereference during a subsequent panic.
- [Low] The early boot initialization function `hyperv_init()` contains unnecessary memory allocation failure handling and cleanup logic for `hv_hostcall_array`.
--

commit 906cd2b30d2ff5884265fe39d28d07e982c9f874
Author: Kameron Carr <[email protected]>

arm64: hyperv: Add per-CPU RSI host call infrastructure for CCA Realms

This commit adds a per-CPU rsi_host_call array allocation for Arm CCA
Realms. It routes Hyper-V hypercalls through the RSI_HOST_CALL interface
instead of HVC.

> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
> index 4fdc26ade1d74..7d536d7fb557e 100644
> --- a/arch/arm64/hyperv/mshyperv.c
> +++ b/arch/arm64/hyperv/mshyperv.c

[ ... ]

> @@ -85,6 +96,20 @@ static int __init hyperv_init(void)
>  	if (!hyperv_detect_via_acpi() && !hyperv_detect_via_smccc())
>  		return 0;
>  
> +	/*
> +	 * The RSI host-call buffers are only ever used when
> +	 * is_realm_world() is true. Skip the allocation on non-Realm
> +	 * guests. A single contiguous array of nr_cpu_ids entries is
> +	 * allocated; each CPU indexes into it by its processor ID.
> +	 */
> +	if (is_realm_world()) {
> +		hv_hostcall_array = kcalloc(nr_cpu_ids,
> +					    sizeof(struct rsi_host_call),
> +					    GFP_KERNEL);

[Severity: High]
The RMM requires the IPA of the host call structure to be strictly 256-byte
aligned for SMC_RSI_HOST_CALL. Since standard slab allocators like kcalloc()
do not guarantee alignment beyond ARCH_KMALLOC_MINALIGN, could this result
in an unaligned physical address being passed to rsi_host_call() in
hv_set_vpreg()?

> +		if (!hv_hostcall_array)
> +			return -ENOMEM;

[Severity: Low]
This isn't a bug, but since hyperv_init() is an early_initcall executed
early in the boot process, is it necessary to handle this allocation failure
gracefully? Early boot memory allocations typically do not need complex
failure paths, as a failure at this stage implies the system cannot boot.

> +	}
> +
>  	/* Setup the guest ID */

[ ... ]

> @@ -125,6 +151,10 @@ static int __init hyperv_init(void)
>  
>  	hyperv_initialized = true;
>  	return 0;
> +
> +free_hostcall_mem:
> +	hv_hostcall_free();
> +	return ret;

[Severity: High]
If hyperv_init() fails here, hv_hostcall_array is freed and set to NULL.
Since the panic notifiers registered earlier in hv_common_init() are not
unregistered on this error path, will this cause a newly introduced NULL
pointer dereference during a later panic?

When the leaked notifier invokes hyperv_report_panic(), it calls
hv_set_vpreg(), which unconditionally accesses the array:

hv_set_vpreg() {
    ...
    hostcall = &hv_hostcall_array[smp_processor_id()];
    memset(hostcall, 0, sizeof(*hostcall));
    ...
}

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3