Re: [PATCH v9 24/25] iommu/arm-smmu-v3-kvm: Enable nesting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] iommu/arm-smmu-v3: fatal EL2 panic on host-supplied invalid STE or ENOMEM
- [Medium] arm-smmu-v3-kvm: Incorrect EL2 WARN_ON usage for hardware timeout
--

commit 5625f7b65b03ea88554c9ce7668b1dabbbb068b1
Author: Mostafa Saleh <smostafa@google.com>

iommu/arm-smmu-v3-kvm: Enable nesting

This commit enables stage-2 translation for the ARM SMMUv3 under pKVM
by allowing the hypervisor to sanitize and control the stage-2 portion
of the Stream Table Entry (STE).

> diff --git a/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3-hyp.c b/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3-hyp.c
> index 01f23fce6be50..3562bb222f53e 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3-hyp.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3-hyp.c
[ ... ]
> @@ -443,6 +443,60 @@ static int smmu_init_cmdq(struct hyp_arm_smmu_v3_device *smmu)
> +static int smmu_attach_stage_2(struct hyp_arm_smmu_v3_device *smmu,
> +			       struct arm_smmu_ste *ste)
> +{
[ ... ]
> +	/* S2 is not advertised, that should never be attempted. */
> +	if ((cfg == STRTAB_STE_0_CFG_S2_TRANS) ||
> +	    (cfg == STRTAB_STE_0_CFG_NESTED))
> +		return -EINVAL;

[Severity: Critical]
Can this error return lead to a hypervisor panic?

When smmu_attach_stage_2() returns -EINVAL, the error propagates to
smmu_filter_command(), which evaluates it and returns true to indicate
the command should be skipped.

The preexisting caller smmu_emulate_cmdq_insert() then processes this
skip condition using WARN_ON:

smmu_emulate_cmdq_insert() {
    ...
    skip = smmu_filter_command(smmu, cmd);
    if (WARN_ON(skip))
        continue;
}

At EL2 under pKVM, WARN_ON expands to a fatal breakpoint. Since the host
can supply an invalid STE configuration that triggers this -EINVAL, could
this allow the host to maliciously crash the hypervisor?

Similarly, allocating the L2 table in smmu_reshadow_ste() can fail and
return -ENOMEM, which also propagates into this same fatal WARN_ON path
during an out-of-memory event.

[ ... ]
> +static void smmu_ste_writer_sync_entry(struct arm_smmu_entry_writer *writer)
> +{
> +	struct smmu_ste_writer *ste_writer =
> +		container_of(writer, struct smmu_ste_writer, writer);
> +	struct arm_smmu_cmd cfgi_cmd = arm_smmu_make_cmd_cfgi_ste(ste_writer->sid, true);
> +
> +	WARN_ON(smmu_send_cmd(ste_writer->smmu, &cfgi_cmd));
> +}

[Severity: Medium]
Is it safe to use WARN_ON for hardware interactions that might time out?

If the SMMU command queue is full and smmu_send_cmd() times out waiting
for it to drain, it will return -ETIMEDOUT. Because WARN_ON is fatal at
EL2 under pKVM, this transforms a transient hardware latency or a heavy
command load into a hypervisor panic.

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