[PATCH v2 2/2] lib: sbi_pmu: reject out-of-bounds counter index in pmu_ctr_write_hw()

[Sebastian Alba Vives]

pmu_ctr_write_hw() writes CSR_MCYCLE + cidx with no check on cidx,
while its sibling pmu_ctr_start_hw() guards the same index space:

	if (cidx >= num_hw_ctrs || cidx == 1)
		return SBI_EINVAL;

A cidx at or above num_hw_ctrs addresses an mhpmcounter the platform
need not implement, and cidx == 1 addresses 0xb01, which is not
assigned. Either one traps in M-mode instead of returning an error.

Give the function the same guard and a return value. The guard is
copied from pmu_ctr_start_hw() rather than limited to the upper
bound, so that the two functions accept exactly the same index space.

Only the call site in sbi_pmu_ctr_cfg_match() propagates the error.
The two call sites inside pmu_ctr_start_hw() run after the identical
guard in that function, so they cannot fail and are left unchanged.

Fixes: 13d40f21d588 ("lib: sbi: Add PMU support")
Signed-off-by: Sebastian Alba Vives <sebasjosue84 at gmail.com>
Reviewed-by: Anup Patel <anup at brainfault.org>
---
 lib/sbi/sbi_pmu.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index c68a675a..96438599 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -389,8 +389,12 @@ static int pmu_ctr_enable_irq_hw(int ctr_idx)
 	return 0;
 }
 
-static void pmu_ctr_write_hw(uint32_t cidx, uint64_t ival)
+static int pmu_ctr_write_hw(uint32_t cidx, uint64_t ival)
 {
+	/* Make sure the counter index lies within the range and is not TM bit */
+	if (cidx >= num_hw_ctrs || cidx == 1)
+		return SBI_EINVAL;
+
 #if __riscv_xlen == 32
 	csr_write_num(CSR_MCYCLE + cidx, 0);
 	csr_write_num(CSR_MCYCLEH + cidx, ival >> BITS_PER_LONG);
@@ -398,6 +402,8 @@ static void pmu_ctr_write_hw(uint32_t cidx, uint64_t ival)
 #else
 	csr_write_num(CSR_MCYCLE + cidx, ival);
 #endif
+
+	return 0;
 }
 
 static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
@@ -980,8 +986,11 @@ skip_match:
 	    event_type == SBI_PMU_EVENT_TYPE_HW_CACHE ||
 	    event_type == SBI_PMU_EVENT_TYPE_HW_RAW ||
 	    event_type == SBI_PMU_EVENT_TYPE_HW_RAW_V2) {
-		if (flags & SBI_PMU_CFG_FLAG_CLEAR_VALUE)
-			pmu_ctr_write_hw(ctr_idx, 0);
+		if (flags & SBI_PMU_CFG_FLAG_CLEAR_VALUE) {
+			ret = pmu_ctr_write_hw(ctr_idx, 0);
+			if (ret)
+				return ret;
+		}
 		if (flags & SBI_PMU_CFG_FLAG_AUTO_START) {
 			ret = pmu_ctr_start_hw(ctr_idx, 0, false);
 			if (ret)
-- 
2.43.0