lib: sbi: clamp sbi_ecall_get_extensions_str buffer offset

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>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260719101125.190314-1-pyudistira519@gmail.com
Signed-off-by: Anup Patel <anup@brainfault.org>
This commit is contained in:
Yudistira Putra
2026-08-31 18:08:50 +05:30
committed by Anup Patel
parent 4e79fd7de5
commit f95648d395
2 changed files with 61 additions and 0 deletions
+2
View File
@@ -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;
+59
View File
@@ -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,
};