[PATCH] lib: sbi: clamp sbi_ecall_get_extensions_str buffer offset

[Anup Patel]

On Sun, Jul 19, 2026 at 3:45 PM Yudistira Putra <pyudistira519 at gmail.com> wrote:
>
> 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 at gmail.com>

LGTM.

Reviewed-by: Anup Patel <anup at brainfault.org>

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup

> ---
>  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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi