[PATCH] lib: sbi: sse: fix shared memory double-fetch in sse_write_attrs

[liutong]

sse_write_attrs() reads the attribute values from the S-mode shared
memory in two passes: first to validate them, then to apply them.
S-mode may write the shared memory between the passes, so a value that
sse_event_set_attr_check() would have rejected can still be applied.
For SBI_SSE_ATTR_PREFERRED_HART this leaves e->hartindex derived from
a hart id that sse_event_set_hart_id_check() refused, which
sbi_sse_enable() then resolves to a NULL scratch.

Copy the attributes into a local array, unmap the shared memory right
away, and run both passes against that copy.

The copy is a fixed size stack array. Its bound is currently guaranteed
by sbi_sse_attr_check() in the only caller. Check attr_count here as
well, so that the bound of the array does not depend on validation done
in another function.

Fixes: c8cdf01d8f3a ("lib: sbi: Add support for Supervisor Software Events extension")
Suggested-by: Himanshu Chauhan <himanshu.chauhan at oss.qualcomm.com>
Signed-off-by: liutong <liutong at iscas.ac.cn>
---
Resent on its own from "[PATCH v3 0/3] lib: sbi: fix shared memory
double-fetch in DBTR and SSE":

  https://lore.kernel.org/opensbi/20260910052958.3365324-4-liutong@iscas.ac.cn/

Himanshu NAKed that series on a debug trigger argument. This one is a
different subsystem, so I would rather it be judged on its own; if the
NAK was meant to cover it too, say so and I will drop it.

The attribute that matters is PREFERRED_HART: winning the race stores a
hart id that sse_event_set_hart_id_check() refused, e->hartindex becomes
-1U, and sbi_sse_enable() then dereferences a NULL scratch. Measured on
f6abdaf2 under QEMU MTTCG, -smp 2: unpatched, won in 6 rounds, and
SSE_ENABLE then takes an M-mode load access fault (mcause=0x5,
mtval=0x540); patched, 20000 rounds with no win.

Unchanged from v3 apart from the commit message, which overstated the
bound check: it covers attr_count only.
 lib/sbi/sbi_sse.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/lib/sbi/sbi_sse.c b/lib/sbi/sbi_sse.c
index 767ea128..b3d35d65 100644
--- a/lib/sbi/sbi_sse.c
+++ b/lib/sbi/sbi_sse.c
@@ -1049,25 +1049,40 @@ static int sse_write_attrs(struct sbi_sse_event *e, uint32_t base_attr_id,
 	unsigned long attr = 0, val;
 	uint32_t id, end_id = base_attr_id + attr_count;
 	unsigned long *attrs = (unsigned long *)input_phys;
+	unsigned long local_attrs[SBI_SSE_ATTR_MAX];
+
+	/*
+	 * sbi_sse_attr_check() in the only caller already rejects a larger
+	 * count, but local_attrs is a fixed size stack array, so do not
+	 * rely on that for its bound.
+	 */
+	if (attr_count > SBI_SSE_ATTR_MAX)
+		return SBI_ERR_INVALID_PARAM;
 
 	sbi_hart_protection_temp_map_range(input_phys, sizeof(unsigned long) * attr_count);
 
+	/*
+	 * Snapshot the attributes so that the values checked below are the
+	 * ones applied, even though S-mode can keep writing the shared
+	 * memory.
+	 */
+	copy_attrs(local_attrs, attrs, attr_count);
+
+	sbi_hart_protection_temp_unmap_range(input_phys, sizeof(unsigned long) * attr_count);
+
 	for (id = base_attr_id; id < end_id; id++) {
-		val = attrs[attr++];
+		val = local_attrs[attr++];
 		ret = sse_event_set_attr_check(e, id, val);
 		if (ret)
-			goto out;
+			return ret;
 	}
 
 	attr = 0;
 	for (id = base_attr_id; id < end_id; id++) {
-		val = attrs[attr++];
+		val = local_attrs[attr++];
 		sse_event_set_attr(e, id, val);
 	}
 
-out:
-	sbi_hart_protection_temp_unmap_range(input_phys, sizeof(unsigned long) * attr_count);
-
 	return ret;
 }
 

base-commit: f6abdaf25dfe0608277fc634f3927c1f184ede6d
-- 
2.34.1