[PATCH] lib: sbi: clamp sbi_ecall_get_extensions_str buffer offset

[Yudistira Putra <pyudistira519@gmail.com>]

sbi_ecall_get_extensions_str() advanced offset by the nominal extension
name length without checking remaining capacity. When the caller buffer
was smaller than the concatenated extension list, offset could pass
exts_str_size, so (exts_str_size - offset) became negative and was
passed to sbi_snprintf() as a large u32, and the trailing NUL write
could step past the caller buffer.

The helper can write beyond a caller-provided destination when the
registered extension list exceeds the supplied capacity.

Mirror the guard already used by sbi_hart_get_extensions_str(): stop
appending when the next name would not fit. Add an SBIUNIT regression
that registers several extensions into a 16-byte buffer with a redzone
and verifies no out-of-bounds write.

Closes: https://github.com/riscv-software-src/opensbi/issues/416
Signed-off-by: Yudistira Putra <pyudistira519@gmail.com>
---
 lib/sbi/sbi_ecall.c            |  2 ++
 lib/sbi/tests/sbi_ecall_test.c | 59 ++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/lib/sbi/sbi_ecall.c b/lib/sbi/sbi_ecall.c
index 745fa31..65c5a55 100644
--- a/lib/sbi/sbi_ecall.c
+++ b/lib/sbi/sbi_ecall.c
@@ -66,6 +66,8 @@ void sbi_ecall_get_extensions_str(char *exts_str, int exts_str_size, bool experi
 	sbi_list_for_each_entry(t, &ecall_exts_list, head) {
 		if (experimental != t->experimental)
 			continue;
+		if (offset + sbi_strlen(t->name) + 1 > exts_str_size)
+			break;
 		sbi_snprintf(exts_str + offset, exts_str_size - offset,
 			     "%s,", t->name);
 		offset = offset + sbi_strlen(t->name) + 1;
diff --git a/lib/sbi/tests/sbi_ecall_test.c b/lib/sbi/tests/sbi_ecall_test.c
index 5b6ce37..f5c553e 100644
--- a/lib/sbi/tests/sbi_ecall_test.c
+++ b/lib/sbi/tests/sbi_ecall_test.c
@@ -40,10 +40,69 @@ static void test_sbi_ecall_register_find_extension(struct sbiunit_test_case *tes
 	SBIUNIT_EXPECT_EQ(test, sbi_ecall_find_extension(SBI_EXT_EXPERIMENTAL_START), NULL);
 }
 
+static void test_sbi_ecall_get_extensions_str_bounds(struct sbiunit_test_case *test)
+{
+	struct sbi_ecall_extension e1 = {
+		.extid_start = SBI_EXT_EXPERIMENTAL_START,
+		.extid_end = SBI_EXT_EXPERIMENTAL_START,
+		.name = "Alpha",
+		.handle = dummy_handler,
+		.experimental = false,
+	};
+	struct sbi_ecall_extension e2 = {
+		.extid_start = SBI_EXT_EXPERIMENTAL_START + 1,
+		.extid_end = SBI_EXT_EXPERIMENTAL_START + 1,
+		.name = "Bravo",
+		.handle = dummy_handler,
+		.experimental = false,
+	};
+	struct sbi_ecall_extension e3 = {
+		.extid_start = SBI_EXT_EXPERIMENTAL_START + 2,
+		.extid_end = SBI_EXT_EXPERIMENTAL_START + 2,
+		.name = "Charli",
+		.handle = dummy_handler,
+		.experimental = false,
+	};
+	char storage[16 + 16];
+	char *buf = storage;
+	char big[128];
+	int i;
+	int found_alpha = 0;
+
+	SBIUNIT_EXPECT_EQ(test, sbi_ecall_register_extension(&e1), 0);
+	SBIUNIT_EXPECT_EQ(test, sbi_ecall_register_extension(&e2), 0);
+	SBIUNIT_EXPECT_EQ(test, sbi_ecall_register_extension(&e3), 0);
+
+	for (i = 16; i < 32; i++)
+		storage[i] = (char)0xA5;
+
+	/* Undersized buffer must not write past the caller-provided size. */
+	sbi_ecall_get_extensions_str(buf, 16, false);
+	SBIUNIT_EXPECT_EQ(test, buf[15], '\0');
+	for (i = 16; i < 32; i++)
+		SBIUNIT_EXPECT_EQ(test, (unsigned char)storage[i], 0xA5);
+
+	/* Negative control: room for the full list, including registered names. */
+	sbi_ecall_get_extensions_str(big, sizeof(big), false);
+	SBIUNIT_EXPECT_NE(test, sbi_strlen(big), 0);
+	for (i = 0; big[i] != '\0'; i++) {
+		if (sbi_strncmp(&big[i], "Alpha", 5) == 0) {
+			found_alpha = 1;
+			break;
+		}
+	}
+	SBIUNIT_EXPECT_EQ(test, found_alpha, 1);
+
+	sbi_ecall_unregister_extension(&e1);
+	sbi_ecall_unregister_extension(&e2);
+	sbi_ecall_unregister_extension(&e3);
+}
+
 static struct sbiunit_test_case ecall_tests[] = {
 	SBIUNIT_TEST_CASE(test_sbi_ecall_version),
 	SBIUNIT_TEST_CASE(test_sbi_ecall_impid),
 	SBIUNIT_TEST_CASE(test_sbi_ecall_register_find_extension),
+	SBIUNIT_TEST_CASE(test_sbi_ecall_get_extensions_str_bounds),
 	SBIUNIT_END_CASE,
 };
 
-- 
2.43.0


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi