Re: [PATCH v9 6/6] KVM: arm64: Zero out the stack initialized data in the FFA handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] EL2 silently accepts a downgrade response from EL3 and incorrectly uses the host's requested version as the negotiated version, leading to version desynchronization.
- [Medium] Host can trigger a reliable EL2 hypervisor panic (Denial of Service) by passing a fake handle to FFA_MEM_FRAG_TX alongside an invalid PFN during rollback.
- [Medium] Optional FF-A 1.1/1.2 interfaces not implemented by pKVM are not masked in `FFA_FEATURES` queries, allowing them to be passed through to EL3.
--

commit 04220564e97901b7aa4aa40ada1444ec9b7ccea7
Author: Sebastian Ene <sebastianene@google.com>

KVM: arm64: Zero out the stack initialized data in the FFA handler
    
This commit initializes the arm_smccc_1_2_regs structure with zeros in
kvm_host_ffa_handler to prevent leaking hypervisor stack data.

Link: https://lore.kernel.org/all/20260616160016.C62C81F000E9@smtp.kernel.org/

> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index 9c96e72e522e5..a327c2bbb6b64 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -880,7 +880,7 @@ static void do_ffa_part_get(struct arm_smccc_1_2_regs *res,
>  
>  bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
>  {
> -	struct arm_smccc_1_2_regs res;
> +	struct arm_smccc_1_2_regs res = {0};

While reviewing this code, I noticed a few pre-existing issues in the same
file.

arch/arm64/kvm/hyp/nvhe/ffa.c:do_ffa_version() {
    ...
    hyp_smccc_1_2_smc(&(struct arm_smccc_1_2_regs) {
        .a0 = FFA_VERSION,
        .a1 = ffa_req_version,
    }, res);
    if ((s32)res->a0 == FFA_RET_NOT_SUPPORTED)
        goto unlock;

    hyp_ffa_version = ffa_req_version;
    ...
}

[Severity: High]
This is a pre-existing issue, but does EL2 silently accept a downgrade
response from EL3 here and incorrectly use the host's requested version as
the negotiated version?

If EL3 accepts but returns an even lower version in res->a0, EL2 ignores it
and sets hyp_ffa_version = ffa_req_version.

Could this version desynchronization between EL2 and EL3 lead to mismatched
memory descriptor parsing and break guest isolation?


arch/arm64/kvm/hyp/nvhe/ffa.c:do_ffa_mem_frag_tx() {
    ...
    ret = ffa_host_share_ranges(buf, nr_ranges);
    if (ret) {
        /*
         * We're effectively aborting the transaction, so we need
         * to restore the global state back to what it was prior to
         * transmission of the first fragment.
         */
        ffa_mem_reclaim(res, handle_lo, handle_hi, 0);
        WARN_ON(res->a0 != FFA_SUCCESS);
        goto out_unlock;
    }
    ...
}

[Severity: Medium]
This isn't a bug introduced by this patch, but can the host trigger a
reliable EL2 hypervisor panic here?

If the host provides an invalid memory range, ffa_host_share_ranges() fails.
In the rollback path, if the host also provided an unauthorized or fake
handle_lo or handle_hi, EL3 rejects the reclaim and returns an error.

Does this cause EL2 to hit the WARN_ON(res->a0 != FFA_SUCCESS), which expands
to a fatal BUG() and causes a denial of service?


arch/arm64/kvm/hyp/nvhe/ffa.c:ffa_call_supported() {
    ...
    /* Optional notification interfaces added in FF-A 1.1 */
    case FFA_NOTIFICATION_BITMAP_CREATE:
    ...
    case FFA_PARTITION_INFO_GET_REGS:
        return false;
    }

    return true;
}

arch/arm64/kvm/hyp/nvhe/ffa.c:do_ffa_features() {
    ...
    switch (id) {
    case FFA_MEM_SHARE:
    ...
        goto out_handled;
    default:
        return false;
    }
    ...
}

[Severity: Medium]
This is a pre-existing issue, but are we missing masking for optional
FF-A 1.1/1.2 interfaces not implemented by pKVM?

ffa_call_supported() omits several optional interfaces such as
FFA_FN64_NOTIFICATION_INFO_GET and FFA_MSG_SEND2, returning true for them.

Then in do_ffa_features(), it returns false for these unhandled queries,
causing kvm_host_ffa_handler() to forward the query to EL3.

Could this incorrectly advertise optional FF-A features to the host that the
hypervisor does not actually implement?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702103848.1647249-1-sebastianene@google.com?part=6