Re: [PATCH v5 1/7] KVM: arm64: Enforce strict SBZ checks in the FF-A proxy

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The strict SBZ check for unused arguments does not ignore the upper 32 bits for SMC32 calls, violating the SMCCC and FF-A specifications.
- [Medium] The FFA_FEATURES handler incorrectly returns FFA_RET_INVALID_PARAMETERS instead of FFA_RET_NOT_SUPPORTED when unused arguments are non-zero.
--

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

KVM: arm64: Enforce strict SBZ checks in the FF-A proxy

Introduce a helper method ffa_check_unused_args_sbz to enforce strict
arguments checking when the hypervisor acts as a relayer between the
host and Trustzone.

> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index 1af722771178a..78bb043b33ee0 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -71,6 +71,20 @@ static u32 hyp_ffa_version;
>  static bool has_version_negotiated;
>  static hyp_spinlock_t version_lock;
>  
> +static bool ffa_check_unused_args_sbz(struct kvm_cpu_context *ctxt, int first_reg)
> +{
> +	DECLARE_REG(u32, func_id, ctxt, 0);
> +	int reg, end_reg;
> +
> +	end_reg = ARM_SMCCC_IS_64(func_id) ? 17 : 7;
> +	for (reg = first_reg; reg <= end_reg; reg++) {
> +		if (cpu_reg(ctxt, reg))

[Severity: High]
Does this correctly handle the upper 32 bits for SMC32 calls?

According to the SMCCC, for SMC32 calls, the upper 32 bits of X registers are
unknown, and the host is not required to clear them. Since cpu_reg() returns
the full 64-bit register value, this might erroneously reject valid host FF-A
SMC32 calls if there is residual data in the upper 32 bits.

> +			return true;
> +	}
> +
> +	return false;
> +}

[ ... ]

> @@ -890,6 +939,11 @@ bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
>  
>  	switch (func_id) {
>  	case FFA_FEATURES:
> +		if (ffa_check_unused_args_sbz(host_ctxt, 3)) {
> +			ffa_to_smccc_res(&res, FFA_RET_INVALID_PARAMETERS);

[Severity: Medium]
Should this return FFA_RET_NOT_SUPPORTED instead?

According to the FF-A specification, if an unused parameter is non-zero in an
FFA_FEATURES call, the implementation must return FFA_RET_NOT_SUPPORTED.
Returning FFA_RET_INVALID_PARAMETERS might violate the FF-A ABI and confuse the
host driver during capability negotiation.

> +			goto out_handled;
> +		}
> +
>  		if (!do_ffa_features(&res, host_ctxt))
>  			return false;
>  		goto out_handled;

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