[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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.