[PATCH v2 03/11] iommu/tegra241-cmdqv: Harden error-map index handling in the error ISR
Nicolin Chen <[email protected]>
| Newsgroups | org.kernel.vger.linux-tegra,dev.linux.lists.iommu,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <25ec919bb10325cac02124babdf4034f2b9dbc23.1784059577.git.nicolinc@nvidia.com> |
tegra241_vintf0_handle_error() reads both 64-bit LVCMDQ error-map registers but used the register-local __ffs64() bit directly as the vintf->lvcmdqs[] index. For the second register that selects the wrong queue instead of 64 * i + bit, clearing the wrong queue's error status. The index is unbounded too: a bit at or beyond num_lvcmdqs_per_vintf would walk the read off vintf->lvcmdqs[]. tegra241_cmdqv_isr() has the same flaw one level up: a VINTF_ERR_MAP bit at or beyond num_vintfs would walk the read off cmdqv->vintfs[]. Use 64 * i + bit for the index and clear the snapshot with the local bit. In both handlers, WARN_ON_ONCE() and skip an out-of-bounds index. Only a malfunctioning device sets such a bit, so the _ONCE form keeps a wedged map from flooding the log. Note that 64 * i + bit is not reachable with the current configuration as a VINTF is pre-assigned with 2 lvcmdqs, this is not treated as bug fix but an defensive hardening. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Nicolin Chen <[email protected]> --- drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c index 5fbc1c85874a5..b89f021ba0b86 100644 --- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c @@ -311,25 +311,30 @@ static void tegra241_vintf_user_handle_error(struct tegra241_vintf *vintf) static void tegra241_vintf0_handle_error(struct tegra241_vintf *vintf) { + struct tegra241_cmdqv *cmdqv = vintf->cmdqv; int i; for (i = 0; i < LVCMDQ_ERR_MAP_NUM_64; i++) { u64 map = readq_relaxed(REG_VINTF(vintf, LVCMDQ_ERR_MAP_64(i))); while (map) { - unsigned long lidx = __ffs64(map); + unsigned long map_bit = __ffs64(map); + unsigned long lidx = 64 * i + map_bit; struct tegra241_vcmdq *vcmdq; u32 gerror; - map &= ~BIT_ULL(lidx); + map &= ~BIT_ULL(map_bit); + /* A bit beyond the count means a HW error; skip it */ + if (WARN_ON_ONCE(lidx >= cmdqv->num_lvcmdqs_per_vintf)) + continue; /* Pairs with smp_store_release() publishing it */ vcmdq = smp_load_acquire(&vintf->lvcmdqs[lidx]); if (!vcmdq) continue; gerror = readl_relaxed(REG_VCMDQ_PAGE0(vcmdq, GERROR)); - __arm_smmu_cmdq_skip_err(&vintf->cmdqv->smmu, &vcmdq->cmdq); + __arm_smmu_cmdq_skip_err(&cmdqv->smmu, &vcmdq->cmdq); writel(gerror, REG_VCMDQ_PAGE0(vcmdq, GERRORN)); } } @@ -381,6 +386,9 @@ static irqreturn_t tegra241_cmdqv_isr(int irq, void *devid) vintf_map &= ~BIT_ULL(idx); + /* A bit beyond the count means a HW error; skip it */ + if (WARN_ON_ONCE(idx >= cmdqv->num_vintfs)) + continue; /* The slot may be published or torn down (NULL'd) concurrently */ vintf = smp_load_acquire(&cmdqv->vintfs[idx]); if (vintf) -- 2.43.0