KVM: arm64: Use acquire/release to communicate FF-A version negotiation
Will Deacon
maintainerPATCH 1/1 KVM: arm64: Use acquire/release to communicate FF-A version negotiation
The pKVM FF-A proxy rejects FF-A requests other than FFA_VERSION until version negotiation is complete, which is signalled by setting the global 'has_version_negotiated' variable.
To avoid excessive locking, this variable is checked directly from kvm_host_ffa_handler() in response to an FF-A call, but this can race against another CPU performing the negotiation and potentially lead to reading a torn value (incredibly unlikely for a 'bool') or problematic re-ordering of the accesses to 'has_version_negotiated' and 'hyp_ffa_version' whereby a stale version number could be read by __do_ffa_mem_xfer().
Use acquire/release primitives when writing 'has_version_negotiated' with the version lock held and when reading without the lock held.
---
Found by code inspection rather than any real issue in practice.
arch/arm64/kvm/hyp/nvhe/ffa.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
arch/arm64/kvm/hyp/nvhe/ffa.c +5 -4
@@ -730,10 +730,10 @@ static void do_ffa_version(struct arm_smccc_res *res, hyp_ffa_version = ffa_req_version; } -if (hyp_ffa_post_init ()) +if (hyp_ffa_post_init ()) { res->a0 =FFA_RET_NOT_SUPPORTED ; -else { - has_version_negotiated = true; + }else { +smp_store_release (&has_version_negotiated, true); res->a0 = hyp_ffa_version; } unlock:
@@ -809,7 +809,8 @@ bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)if (!is_ffa_call (func_id))return false; -if (!has_version_negotiated && func_id !=FFA_VERSION ) { +if (func_id !=FFA_VERSION && + !smp_load_acquire (&has_version_negotiated)) {ffa_to_smccc_error (&res,FFA_RET_INVALID_PARAMETERS ); goto out_handled; }
Signature · 1 line
2.49.0.504.g3bcea36a83-goog
Oliver Upton
AppliedQuoted 12 lines · Will Deacon
The pKVM FF-A proxy rejects FF-A requests other than
FFA_VERSIONuntil version negotiation is complete, which is signalled by setting the global 'has_version_negotiated' variable.To avoid excessive locking, this variable is checked directly from
kvm_host_ffa_handler()in response to an FF-A call, but this can race against another CPU performing the negotiation and potentially lead to reading a torn value (incredibly unlikely for a 'bool') or problematic re-ordering of the accesses to 'has_version_negotiated' and 'hyp_ffa_version' whereby a stale version number could be read by__do_ffa_mem_xfer().
Quoted 1 line
[...]
Applied to fixes, thanks!
[1/1] KVM: arm64: Use acquire/release to communicate FF-A version negotiation
https://git.kernel.org/kvmarm/kvmarm/c/a344e258acb0
--
Best,
Oliver