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

[Mostafa Saleh <smostafa@google.com>]

On Thu, Oct 15, 2026 at 08:34:42PM +0000, sashiko-bot@kernel.org wrote:
> 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.

Yes, that is by design as replied previously.

> 
> [ ... ]
> > +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.

As discussed in the last patch.

Thanks,
Mostafa

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