Re: [PATCH v5 5/7] KVM: arm64: Support FFA_NOTIFICATION_SET in host handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `FFA_NOTIFICATION_SET` handler incorrectly uses `GENMASK(15, 2)` for flags validation, failing to enforce the MBZ requirement for bits [31:16].
--

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

KVM: arm64: Support FFA_NOTIFICATION_SET in host handler

This commit adds support for proxying the FFA_NOTIFICATION_SET message from
the host to TrustZone in the pKVM hypervisor. It also implements enforcement
of MBZ/SBZ fields for the hypercall arguments.

> 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

[ ... ]

> @@ -977,6 +977,32 @@ 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 (ffa_check_unused_args_sbz(ctxt, 5)) {
> +		ffa_to_smccc_res(res, FFA_RET_INVALID_PARAMETERS);
> +		return;
> +	}
> +
> +	if (flags & GENMASK(15, 2)) {
> +		ffa_to_smccc_res(res, FFA_RET_INVALID_PARAMETERS);
> +		return;
> +	}

[Severity: Medium]
Does this flags validation completely enforce the MBZ requirement?

The commit message mentions enforcing MBZ/SBZ fields, but checking against
GENMASK(15, 2) appears to leave bits [31:16] unchecked. If an untrusted
host invokes the FFA_NOTIFICATION_SET hypercall with non-zero values in the
upper bits of the flags register (w2), it looks like this validation would
evaluate to false and allow the malformed arguments to be proxied to EL3.

Should this mask be expanded to include GENMASK(31, 16)?

> +
> +	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=5