Am 17. September 2026 11:45:31 MESZ schrieb "Vincent Stehlé"
<[email protected]>:
>The new_string() and set_string() HII string protocol functions keep
>using old pointers after calling realloc(), which may have moved.
>
>The set_string() function also assumes that reallocation only grew the
>array by one element when computing the start of the memory to clear,
>and forgets to update nstrings to the new table size after reallocation.
>
>Repair and factorize the reallocation code in a new grow_string_table()
>function to fix those issues.
Should we add a test in U-Boot and possibly in SCT?
Best regards
Heinrich
>
>Fixes: c9bfb22296b0 ("efi_loader: Initial HII database protocols")
>Signed-off-by: Vincent Stehlé <[email protected]>
>Cc: Heinrich Schuchardt <[email protected]>
>Cc: Ilias Apalodimas <[email protected]>
>Cc: Tom Rini <[email protected]>
>---
>This can be tested on the sandbox with the following command:
>
> ./u-boot -T -c 'setenv efi_selftest HII database protocols; bootefi selftest'
>
>While the unit test does cause reallocation with new_string(), it does not
>with set_string().
>
>Further testing can be done by forcing stbl->strings = NULL, just after
>calling realloc(stbl->strings, ...).
>---
> lib/efi_loader/efi_hii.c | 64 +++++++++++++++++++++++++++---------------------
> 1 file changed, 36 insertions(+), 28 deletions(-)
>
>diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c
>index 7bf51ad43d1..90dff91e5d4 100644
>--- a/lib/efi_loader/efi_hii.c
>+++ b/lib/efi_loader/efi_hii.c
>@@ -822,6 +822,37 @@ static bool language_match(char *language, char
>*languages)
> return false;
> }
>
>+/**
>+ * grow_string_table() - Grow a string table if necessary.
>+ * @stbl: String table to grow.
>+ * @n: Minimum table size desired.
>+ *
>+ * Make sure a string table can contain at least n elements.
>+ * If the table is too small, reallocate the memory and clear the new
>elements.
>+ *
>+ * Return: false in case of reallocation error, true otherwise.
>+ * In case of error, the original string table is left untouched.
>+ * In case of success, stbl->strings and stbl->nstrings may have been updated.
>+ */
>+static bool grow_string_table(struct efi_string_table *stbl, u32 n)
>+{
>+ void *buf;
>+ u32 old_n = stbl->nstrings;
>+
>+ if (old_n >= n)
>+ return true;
>+
>+ buf = realloc(stbl->strings, sizeof(stbl->strings[0]) * n);
>+ if (!buf)
>+ return false;
>+
>+ stbl->strings = buf;
>+ stbl->nstrings = n;
>+ memset(&stbl->strings[old_n], 0, sizeof(stbl->strings[0]) * (n -
>old_n));
>+
>+ return true;
>+}
>+
> static efi_status_t EFIAPI
> new_string(const struct efi_hii_string_protocol *this,
> efi_hii_handle_t package_list,
>@@ -847,23 +878,12 @@ new_string(const struct efi_hii_string_protocol *this,
> list_for_each_entry(stbl, &hii->string_tables, link) {
> if (language_match((char *)language, stbl->language)) {
> efi_string_id_t new_id;
>- void *buf;
> efi_string_t str;
>
> new_id = ++hii->max_string_id;
>- if (stbl->nstrings < new_id) {
>- buf = realloc(stbl->strings,
>- sizeof(stbl->strings[0])
>- * new_id);
>- if (!buf)
>- return EFI_EXIT(EFI_OUT_OF_RESOURCES);
>-
>- memset(&stbl->strings[stbl->nstrings], 0,
>- (new_id - stbl->nstrings)
>- * sizeof(stbl->strings[0]));
>- stbl->strings = buf;
>- stbl->nstrings = new_id;
>- }
>+
>+ if (!grow_string_table(stbl, new_id))
>+ return EFI_EXIT(EFI_OUT_OF_RESOURCES);
>
> str = u16_strdup(string);
> if (!str)
>@@ -957,20 +977,8 @@ set_string(const struct efi_hii_string_protocol *this,
> if (hii->max_string_id < string_id)
> return EFI_EXIT(EFI_NOT_FOUND);
>
>- if (stbl->nstrings < string_id) {
>- void *buf;
>-
>- buf = realloc(stbl->strings,
>- string_id
>- * sizeof(stbl->strings[0]));
>- if (!buf)
>- return EFI_EXIT(EFI_OUT_OF_RESOURCES);
>-
>- memset(&stbl->strings[string_id - 1], 0,
>- (string_id - stbl->nstrings)
>- * sizeof(stbl->strings[0]));
>- stbl->strings = buf;
>- }
>+ if (!grow_string_table(stbl, string_id))
>+ return EFI_EXIT(EFI_OUT_OF_RESOURCES);
>
> str = u16_strdup(string);
> if (!str)
>
>---
>base-commit: 2c7eaee7b3422ce7c84e892abfe16e1afce42e83
>change-id: 20260916-ptr-670be74dcbd3
>
>Best regards,
>--
>Vincent.
>
