Thank you for your contribution! Sashiko AI review found 3 potential issue(s) 
to consider:
- [High] NMI/FIQ Reentrancy Data Corruption in Realm Hypercalls
- [High] Uninitialized Access / Missing NULL Check on Per-CPU Hostcall Pointer
- [High] Hardware ABI Violation / Unsafe Memory Allocation for rsi_host_call
--

commit 17e33243e24b87deba9b6417fd0671d6c8f9140b
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..1759998ef2667 100644
> --- a/arch/arm64/hyperv/hv_core.c
> +++ b/arch/arm64/hyperv/hv_core.c
[ ... ]
> @@ -25,12 +26,32 @@ u64 hv_do_hypercall(u64 control, void *input, void 
> *output)
>       struct arm_smccc_res    res;
>       u64                     input_address;
>       u64                     output_address;
> +     struct rsi_host_call *hostcall;
> +     unsigned long flags;
> +     u64 ret;
>  
>       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()) {
> +             local_irq_save(flags);
> +             hostcall = *this_cpu_ptr(hyperv_pcpu_hostcall_struct);
> +             memset(hostcall, 0, sizeof(*hostcall));

[Severity: High]
Could hostcall be NULL here?

If a hypercall is issued before the per-CPU structure is allocated for the
local CPU (such as during early boot, early AP bring-up, or broadcast TLB
flushes), won't passing this to memset() cause a NULL pointer dereference?

[Severity: High]
Since hypercall arguments are now passed using a shared per-CPU memory
structure instead of registers, is there a risk of reentrancy corruption?

local_irq_save() disables IRQs, but it does not mask NMIs or synchronous
exceptions. If an NMI or exception interrupts this code after the memset()
and issues another hypercall (like a crash IPI via hv_do_fast_hypercall8()),
won't the nested hypercall overwrite this shared hostcall buffer?

When the exception returns, the interrupted hypercall would then execute
using the corrupted arguments or read the wrong return status.

> +             hostcall->gprs[0] = HV_FUNC_ID;
> +             hostcall->gprs[1] = control;
> +             hostcall->gprs[2] = input_address;
> +             hostcall->gprs[3] = output_address;
> +
> +             if (rsi_host_call(virt_to_phys(hostcall)) == RSI_SUCCESS)

[Severity: High]
Is the 256-byte alignment required by the RMM guaranteed for this physical
address?

While the struct declaration might include an alignment attribute, dynamic
allocations don't always guarantee 256-byte physical alignment. For
instance, standard allocations only guarantee alignment up to
ARCH_KMALLOC_MINALIGN, and debugging features like CONFIG_SLUB_DEBUG can
shift object payloads, breaking natural power-of-two alignment.

Would passing an unaligned physical address here cause the RMM to reject
the call or inject a synchronous alignment fault into the Realm guest?

> +                     ret = hostcall->gprs[0];
> +             else
> +                     ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
> +             local_irq_restore(flags);
> +             return ret;
> +     }
> +
>       arm_smccc_1_1_hvc(HV_FUNC_ID, control, input_address,
>                         output_address, &res);
>       return res.a0;

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=5

Reply via email to