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
