Re: [PATCH v5 3/7] KVM: arm64: Support FFA_NOTIFICATION_BIND in host handler

[sashiko-bot@kernel.org]

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

New issues:
- [High] FFA_NOTIFICATION_BIND rejects per-vCPU bindings for VCPU IDs > 0 due to unconditional unused argument check on w5.

Pre-existing issues:
- [High] `ffa_check_unused_args_sbz()` validates the full 64-bit register for SMC32 calls, violating the SMCCC specification.
--

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

KVM: arm64: Support FFA_NOTIFICATION_BIND in host handler

Verify the arguments of the FF-A notification bind call and forward the
message to Trustzone.

> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c

[ ... ]

> @@ -930,6 +931,32 @@ static void do_ffa_notif_bitmap(struct arm_smccc_1_2_regs *res,
>  	hyp_smccc_1_2_smc(args, res);
>  }
>  
> +static void do_ffa_notif_bind(struct arm_smccc_1_2_regs *res,
> +			      struct kvm_cpu_context *ctxt)
> +{
> +	DECLARE_REG(u32, endp_id, ctxt, 1);
> +	DECLARE_REG(u32, flags, ctxt, 2);
> +	struct arm_smccc_1_2_regs *args;
> +
> +	if (ffa_check_unused_args_sbz(ctxt, 5)) {

[Severity: High]
Does this unconditionally reject per-vCPU bindings for VCPU IDs greater
than 0?

According to the FF-A 1.1 specification, when Flags bit 0 is set to 1,
w5 contains the VCPU ID. This means w5 is not an unused argument when
binding a per-vCPU notification.

Could we skip the sbz check for w5 when the per-vCPU flag is set?

[Severity: High]
This is a pre-existing issue, but I noticed a problem with how
ffa_check_unused_args_sbz() processes SMC32 calls like this one.

When it checks the registers:

static bool ffa_check_unused_args_sbz(struct kvm_cpu_context *ctxt, int first_reg)
{
        ...
        for (reg = first_reg; reg <= end_reg; reg++) {
                if (cpu_reg(ctxt, reg))
                        return true;
        }
        ...
}

It reads the full 64-bit register using cpu_reg(). However, the SMCCC 1.2
specification states that the upper 32 bits of W0-W7 are unpredictable for
SMC32 calls.

Will this cause KVM to spuriously reject valid SMC32 FF-A calls if the
host OS leaves non-zero garbage in the upper 32 bits of the unused
registers?

> +		ffa_to_smccc_res(res, FFA_RET_INVALID_PARAMETERS);
> +		return;
> +	}
> +
> +	if (FIELD_GET(FFA_NOTIF_RECEIVER_ENDP_MASK, endp_id) != HOST_FFA_ID) {
> +		ffa_to_smccc_res(res, FFA_RET_INVALID_PARAMETERS);
> +		return;
> +	}
> +
> +	if (flags > 1) {
> +		ffa_to_smccc_res(res, FFA_RET_INVALID_PARAMETERS);
> +		return;
> +	}
> +
> +	args = (void *)&ctxt->regs.regs[0];
> +	hyp_smccc_1_2_smc(args, res);
> +}

[ ... ]

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