[PATCH v2 1/1] efi_loader: fix HII GetString()

[PATCH v2 1/1] efi_loader: fix HII GetString()

[Heinrich Schuchardt]

* Add missing conformance tests.
* Return EFI_INVALID_LANGUAGE if a string does not exist in the requested
  language.
* Handle querying the string size with *string_size == 0 and string = NULL.

Fixes: c9bfb22296b0 ("efi_loader: Initial HII database protocols")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v2:
	string_id == 0 is not used by our code. Return EFI_NOT_FOUND.
---
 lib/efi_loader/efi_hii.c | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c
index 90dff91e5d4..dc02d7d2ba6 100644
--- a/lib/efi_loader/efi_hii.c
+++ b/lib/efi_loader/efi_hii.c
@@ -910,41 +910,55 @@ get_string(const struct efi_hii_string_protocol *this,
 {
 	struct efi_hii_packagelist *hii = package_list;
 	struct efi_string_table *stbl;
+	efi_status_t ret;
 
 	EFI_ENTRY("%p, \"%s\", %p, %u, %p, %p, %p", this, language,
 		  package_list, string_id, string, string_size,
 		  string_font_info);
 
-	if (!package_list || !efi_hii_packagelist_exists(package_list))
-		return EFI_EXIT(EFI_NOT_FOUND);
+	if (!language || !string_size || (*string_size && !string)) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	if (!package_list ||
+	    !efi_hii_packagelist_exists(package_list) ||
+	    !string id ||
+	    string_id > hii->max_string_id) {
+		ret = EFI_NOT_FOUND;
+		goto out;
+	}
 
+	ret = EFI_INVALID_LANGUAGE;
 	list_for_each_entry(stbl, &hii->string_tables, link) {
 		if (language_match((char *)language, stbl->language)) {
 			efi_string_t str;
 			size_t len;
 
 			if (stbl->nstrings < string_id)
-				return EFI_EXIT(EFI_NOT_FOUND);
+				goto out;
 
 			str = stbl->strings[string_id - 1].string;
 			if (str) {
 				len = u16_strsize(str);
 				if (*string_size < len) {
 					*string_size = len;
-
-					return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
+					ret = EFI_BUFFER_TOO_SMALL;
+					goto out;
 				}
-				memcpy(string, str, len);
 				*string_size = len;
+				if (string)
+					memcpy(string, str, len);
+				ret = EFI_SUCCESS;
+				goto out;
 			} else {
-				return EFI_EXIT(EFI_NOT_FOUND);
+				goto out;
 			}
-
-			return EFI_EXIT(EFI_SUCCESS);
 		}
 	}
 
-	return EFI_EXIT(EFI_NOT_FOUND);
+out:
+	return EFI_EXIT(ret);
 }
 
 static efi_status_t EFIAPI
-- 
2.53.0

Re: [PATCH v2 1/1] efi_loader: fix HII GetString()

[Vincent Stehlé]

On Mon, Oct 12, 2026 at 09:10:53PM +0200, Heinrich Schuchardt wrote:
> * Add missing conformance tests.

Hi Heinrich,

Thank you for continuing the work on the HII protocols.
I do not see those additional tests in this patch.

Also, one comment below.

Best regards,
Vincent.

> * Return EFI_INVALID_LANGUAGE if a string does not exist in the requested
>   language.
> * Handle querying the string size with *string_size == 0 and string = NULL.
> 
> Fixes: c9bfb22296b0 ("efi_loader: Initial HII database protocols")
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
> 	string_id == 0 is not used by our code. Return EFI_NOT_FOUND.
> ---
>  lib/efi_loader/efi_hii.c | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c
> index 90dff91e5d4..dc02d7d2ba6 100644
> --- a/lib/efi_loader/efi_hii.c
> +++ b/lib/efi_loader/efi_hii.c
> @@ -910,41 +910,55 @@ get_string(const struct efi_hii_string_protocol *this,
>  {
>  	struct efi_hii_packagelist *hii = package_list;
>  	struct efi_string_table *stbl;
> +	efi_status_t ret;
>  
>  	EFI_ENTRY("%p, \"%s\", %p, %u, %p, %p, %p", this, language,
>  		  package_list, string_id, string, string_size,
>  		  string_font_info);
>  
> -	if (!package_list || !efi_hii_packagelist_exists(package_list))
> -		return EFI_EXIT(EFI_NOT_FOUND);
> +	if (!language || !string_size || (*string_size && !string)) {
> +		ret = EFI_INVALID_PARAMETER;
> +		goto out;
> +	}
> +
> +	if (!package_list ||
> +	    !efi_hii_packagelist_exists(package_list) ||
> +	    !string id ||

Missing "_" .......^

> +	    string_id > hii->max_string_id) {
> +		ret = EFI_NOT_FOUND;
> +		goto out;
> +	}
>  
> +	ret = EFI_INVALID_LANGUAGE;
>  	list_for_each_entry(stbl, &hii->string_tables, link) {
>  		if (language_match((char *)language, stbl->language)) {
>  			efi_string_t str;
>  			size_t len;
>  
>  			if (stbl->nstrings < string_id)
> -				return EFI_EXIT(EFI_NOT_FOUND);
> +				goto out;
>  
>  			str = stbl->strings[string_id - 1].string;
>  			if (str) {
>  				len = u16_strsize(str);
>  				if (*string_size < len) {
>  					*string_size = len;
> -
> -					return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
> +					ret = EFI_BUFFER_TOO_SMALL;
> +					goto out;
>  				}
> -				memcpy(string, str, len);
>  				*string_size = len;
> +				if (string)
> +					memcpy(string, str, len);
> +				ret = EFI_SUCCESS;
> +				goto out;
>  			} else {
> -				return EFI_EXIT(EFI_NOT_FOUND);
> +				goto out;
>  			}
> -
> -			return EFI_EXIT(EFI_SUCCESS);
>  		}
>  	}
>  
> -	return EFI_EXIT(EFI_NOT_FOUND);
> +out:
> +	return EFI_EXIT(ret);
>  }
>  
>  static efi_status_t EFIAPI
> -- 
> 2.53.0
> 

-- 
Vincent.