Re: [PATCH v8 4/7] KVM: arm64: Support FFA_NOTIFICATION_SET in host handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `FFA_NOTIFICATION_SET` handler incorrectly uses `GENMASK(15, 2)` to validate the `flags` parameter, failing to check the upper 16 bits.
- [Medium] `do_ffa_notif_set` fails to validate that unused SMC arguments are zero (SBZ) before proxying the call to EL3.
--

--- Patch [4]: [PATCH v8 4/7] KVM: arm64: Support FFA_NOTIFICATION_SET in host handler ---
commit baa4efd6afc2d0d38d7b2978ab35d6925fbb388e
Author: Sebastian Ene <sebastianene@google.com>

KVM: arm64: Support FFA_NOTIFICATION_SET in host handler

This patch allows FF-A notification SET messages to be proxied from the
pKVM hypervisor to Trustzone and verifies the arguments.

> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index d52d7c4d5e7f5..2bb16aa414f9f 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c

[ ... ]

> @@ -917,6 +917,27 @@ static void do_ffa_notif_unbind(struct arm_smccc_1_2_regs *res,
>  	hyp_smccc_1_2_smc(args, res);
>  }
>  
> +static void do_ffa_notif_set(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 (FIELD_GET(FFA_NOTIF_SENDER_ENDP_MASK, endp_id) != HOST_FFA_ID) {
> +		ffa_to_smccc_res(res, FFA_RET_INVALID_PARAMETERS);
> +		return;
> +	}
> +
> +	if (flags & GENMASK(15, 2)) {

[Severity: Medium]
Does this mask adequately validate the upper bits of the flags parameter?

Because GENMASK(15, 2) is used, bits 31:16 of the flags parameter passed in
W2 are left unvalidated. If an untrusted host issues the hypercall with
non-zero values in these upper 16 bits, they will be forwarded directly to EL3.

> +		ffa_to_smccc_res(res, FFA_RET_INVALID_PARAMETERS);
> +		return;
> +	}
> +
> +	args = (void *)&ctxt->regs.regs[0];

[Severity: Medium]
Should this function validate that unused SMC arguments are zero before
proxying the call to EL3? 

Other handlers in this file call ffa_check_unused_args_sbz() to enforce this
behavior. Without that check, if the untrusted host passes arbitrary data in
unused registers, that data will be forwarded verbatim to the Secure Payload
Manager, potentially providing a fuzzing channel against EL3.

> +	hyp_smccc_1_2_smc(args, res);
> +}

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