U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] efi: fix and test hii keyboard layout pointer computation
@ 2026-05-12 17:40 Vincent Stehlé
  2026-05-12 17:40 ` [PATCH 1/2] efi_loader: fix " Vincent Stehlé
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Vincent Stehlé @ 2026-05-12 17:40 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, Tom Rini, AKASHI Takahiro,
	Vincent Stehlé

- Fix one scaled pointer arithmetic bug in EFI Loader HII implementation.
- Enhance the existing EFI HII unit test to catch the bug.

The unit test can run on the sandbox with the following command:

  ./u-boot -T -c "setenv efi_selftest HII database protocols; \
		  bootefi selftest"

Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
---
Vincent Stehlé (2):
      efi_loader: fix hii keyboard layout pointer computation
      efi_selftest: test hii keyboard layouts more

 lib/efi_loader/efi_hii.c                 |  3 +-
 lib/efi_selftest/efi_selftest_hii.c      | 67 ++++++++++++++++++--------------
 lib/efi_selftest/efi_selftest_hii_data.c | 12 ++++++
 3 files changed, 51 insertions(+), 31 deletions(-)
---
base-commit: 5732bd0f457b4c671e46574d64d4acb099c0f0a5
change-id: 20260512-layout-pointer-29d1cd52c070

Best regards,
-- 
Vincent.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] efi_loader: fix hii keyboard layout pointer computation
  2026-05-12 17:40 [PATCH 0/2] efi: fix and test hii keyboard layout pointer computation Vincent Stehlé
@ 2026-05-12 17:40 ` Vincent Stehlé
  2026-05-12 17:40 ` [PATCH 2/2] efi_selftest: test hii keyboard layouts more Vincent Stehlé
  2026-05-12 19:05 ` [PATCH 0/2] efi: fix and test hii keyboard layout pointer computation Heinrich Schuchardt
  2 siblings, 0 replies; 4+ messages in thread
From: Vincent Stehlé @ 2026-05-12 17:40 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, Tom Rini, AKASHI Takahiro,
	Vincent Stehlé

The EFI_HII_KEYBOARD_LAYOUT field `layout_length' is expressed in bytes,
but we add it to the `layout' pointer with (scaled) pointer arithmetic.
When adding an HII keyboard package with multiple keyboard layouts, this
results in only the first layout being added correctly; fix it.

Fixes: 8d3b77e36e10 ("efi: hii: add keyboard layout package support")
Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: AKASHI Takahiro <akashi.tkhro@gmail.com>
---
 lib/efi_loader/efi_hii.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c
index 330d7c5830b..7bf51ad43d1 100644
--- a/lib/efi_loader/efi_hii.c
+++ b/lib/efi_loader/efi_hii.c
@@ -324,7 +324,8 @@ add_keyboard_package(struct efi_hii_packagelist *hii,
 		list_add_tail(&layout_data->link_sys,
 			      &efi_keyboard_layout_list);
 
-		layout += layout_length;
+		layout = (struct efi_hii_keyboard_layout *)
+			 ((uintptr_t)layout + layout_length);
 	}
 
 	list_add_tail(&package_data->link, &hii->keyboard_packages);

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] efi_selftest: test hii keyboard layouts more
  2026-05-12 17:40 [PATCH 0/2] efi: fix and test hii keyboard layout pointer computation Vincent Stehlé
  2026-05-12 17:40 ` [PATCH 1/2] efi_loader: fix " Vincent Stehlé
@ 2026-05-12 17:40 ` Vincent Stehlé
  2026-05-12 19:05 ` [PATCH 0/2] efi: fix and test hii keyboard layout pointer computation Heinrich Schuchardt
  2 siblings, 0 replies; 4+ messages in thread
From: Vincent Stehlé @ 2026-05-12 17:40 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, Tom Rini, AKASHI Takahiro,
	Vincent Stehlé

The HII database test for keyboard layouts register two package lists with
two keyboard layouts each, but the test verifies only the GUID of the first
keyboard layout.
This does not catch the bugs happening with the keyboard layouts after the
first one in a package.

Verify all the keyboard layout GUIDs in the unit test to prevent this.

Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Tom Rini <trini@konsulko.com>
---
 lib/efi_selftest/efi_selftest_hii.c      | 67 ++++++++++++++++++--------------
 lib/efi_selftest/efi_selftest_hii_data.c | 12 ++++++
 2 files changed, 49 insertions(+), 30 deletions(-)

diff --git a/lib/efi_selftest/efi_selftest_hii.c b/lib/efi_selftest/efi_selftest_hii.c
index 228dc296950..fdbb08fb417 100644
--- a/lib/efi_selftest/efi_selftest_hii.c
+++ b/lib/efi_selftest/efi_selftest_hii.c
@@ -452,8 +452,7 @@ out:
  * test_hii_database_get_keyboard_layout() - test retrieval of keyboard layout
  *
  * This test adds two package lists, each of which has two keyboard layouts
- * and then tries to get a handle to keyboard layout with a specific guid
- * and the current one.
+ * and then tries to get a handle to every keyboard layout and the current one.
  *
  * @Return:     status code
  */
@@ -463,7 +462,11 @@ static int test_hii_database_get_keyboard_layout(void)
 	struct efi_hii_keyboard_layout *kb_layout;
 	u16 kb_layout_size;
 	efi_status_t ret;
-	int result = EFI_ST_FAILURE;
+	int result = EFI_ST_FAILURE, i;
+	static efi_guid_t *const kb_layout_guids[] = {
+		&kb_layout_guid11, &kb_layout_guid12,
+		&kb_layout_guid21, &kb_layout_guid22
+	};
 
 	PRINT_TESTNAME;
 	ret = hii_database_protocol->new_package_list(hii_database_protocol,
@@ -484,33 +487,37 @@ static int test_hii_database_get_keyboard_layout(void)
 		goto out;
 	}
 
-	/* specific keyboard_layout(guid11) */
-	kb_layout = NULL;
-	kb_layout_size = 0;
-	ret = hii_database_protocol->get_keyboard_layout(hii_database_protocol,
-			&kb_layout_guid11, &kb_layout_size, kb_layout);
-	if (ret != EFI_BUFFER_TOO_SMALL) {
-		efi_st_error("get_keyboard_layout returned %u\n",
-			     (unsigned int)ret);
-		goto out;
-	}
-	ret = boottime->allocate_pool(EFI_LOADER_DATA, kb_layout_size,
-				      (void **)&kb_layout);
-	if (ret != EFI_SUCCESS) {
-		efi_st_error("AllocatePool failed\n");
-		goto out;
-	}
-	ret = hii_database_protocol->get_keyboard_layout(hii_database_protocol,
-			&kb_layout_guid11, &kb_layout_size, kb_layout);
-	if (ret != EFI_SUCCESS) {
-		efi_st_error("get_keyboard_layout returned %u\n",
-			     (unsigned int)ret);
-		goto out;
-	}
-	ret = boottime->free_pool(kb_layout);
-	if (ret != EFI_SUCCESS) {
-		efi_st_error("FreePool failed\n");
-		goto out;
+	/* Verify all keyboard layouts */
+	for (i = 0; i < ARRAY_SIZE(kb_layout_guids); i++) {
+		efi_guid_t *kb_layout_guid = kb_layout_guids[i];
+
+		kb_layout = NULL;
+		kb_layout_size = 0;
+		ret = hii_database_protocol->get_keyboard_layout(hii_database_protocol,
+				kb_layout_guid, &kb_layout_size, kb_layout);
+		if (ret != EFI_BUFFER_TOO_SMALL) {
+			efi_st_error("get_keyboard_layout returned %u\n",
+				     (unsigned int)ret);
+			goto out;
+		}
+		ret = boottime->allocate_pool(EFI_LOADER_DATA, kb_layout_size,
+					      (void **)&kb_layout);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("AllocatePool failed\n");
+			goto out;
+		}
+		ret = hii_database_protocol->get_keyboard_layout(hii_database_protocol,
+				kb_layout_guid, &kb_layout_size, kb_layout);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("get_keyboard_layout returned %u\n",
+				     (unsigned int)ret);
+			goto out;
+		}
+		ret = boottime->free_pool(kb_layout);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("FreePool failed\n");
+			goto out;
+		}
 	}
 
 	/* current */
diff --git a/lib/efi_selftest/efi_selftest_hii_data.c b/lib/efi_selftest/efi_selftest_hii_data.c
index 5fc890112b4..2d29bb045d3 100644
--- a/lib/efi_selftest/efi_selftest_hii_data.c
+++ b/lib/efi_selftest/efi_selftest_hii_data.c
@@ -428,6 +428,18 @@ static efi_guid_t kb_layout_guid11 =
 	EFI_GUID(0x8d40e495, 0xe2aa, 0x4c6f,
 		 0x89, 0x70, 0x68, 0x85, 0x09, 0xee, 0xc7, 0xd2);
 
+static efi_guid_t kb_layout_guid12 =
+	EFI_GUID(0x2ae60b3e, 0xb9d6, 0x49d8,
+		 0x9a, 0x16, 0xc2, 0x48, 0xf1, 0xeb, 0xa8, 0xdb);
+
+static efi_guid_t kb_layout_guid21 =
+	EFI_GUID(0xe0f56a1f, 0xdf6b, 0x4a7e,
+		 0xa3, 0x9a, 0xe7, 0xa5, 0x19, 0x15, 0x45, 0xd6);
+
+static efi_guid_t kb_layout_guid22 =
+	EFI_GUID(0x47be6ac9, 0x54cc, 0x46f9,
+		 0xa2, 0x62, 0xd5, 0x3b, 0x25, 0x6a, 0x0c, 0x34);
+
 static efi_guid_t package_guid =
 	EFI_GUID(0x0387c95a, 0xd703, 0x2346,
 		 0xb2, 0xab, 0xd0, 0xc7, 0xdd, 0x90, 0x44, 0xf8);

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 0/2] efi: fix and test hii keyboard layout pointer computation
  2026-05-12 17:40 [PATCH 0/2] efi: fix and test hii keyboard layout pointer computation Vincent Stehlé
  2026-05-12 17:40 ` [PATCH 1/2] efi_loader: fix " Vincent Stehlé
  2026-05-12 17:40 ` [PATCH 2/2] efi_selftest: test hii keyboard layouts more Vincent Stehlé
@ 2026-05-12 19:05 ` Heinrich Schuchardt
  2 siblings, 0 replies; 4+ messages in thread
From: Heinrich Schuchardt @ 2026-05-12 19:05 UTC (permalink / raw)
  To: Vincent Stehlé, u-boot; +Cc: Ilias Apalodimas

Am 12. Mai 2026 19:40:32 MESZ schrieb "Vincent Stehlé" <vincent.stehle@arm.com>:
>- Fix one scaled pointer arithmetic bug in EFI Loader HII implementation.
>- Enhance the existing EFI HII unit test to catch the bug.
>
>The unit test can run on the sandbox with the following command:
>
>  ./u-boot -T -c "setenv efi_selftest HII database protocols; \
>		  bootefi selftest"
>
>Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
>---
>Vincent Stehlé (2):
>      efi_loader: fix hii keyboard layout pointer computation
>      efi_selftest: test hii keyboard layouts more
>
> lib/efi_loader/efi_hii.c                 |  3 +-
> lib/efi_selftest/efi_selftest_hii.c      | 67 ++++++++++++++++++--------------
> lib/efi_selftest/efi_selftest_hii_data.c | 12 ++++++
> 3 files changed, 51 insertions(+), 31 deletions(-)
>---
>base-commit: 5732bd0f457b4c671e46574d64d4acb099c0f0a5
>change-id: 20260512-layout-pointer-29d1cd52c070
>
>Best regards,

Hello Vincent,

Off the list:

Did you find any real use for HII protocols in U-Boot except for running the SCT?

Are HII based EFI applications a thing?

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-12 19:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12 17:40 [PATCH 0/2] efi: fix and test hii keyboard layout pointer computation Vincent Stehlé
2026-05-12 17:40 ` [PATCH 1/2] efi_loader: fix " Vincent Stehlé
2026-05-12 17:40 ` [PATCH 2/2] efi_selftest: test hii keyboard layouts more Vincent Stehlé
2026-05-12 19:05 ` [PATCH 0/2] efi: fix and test hii keyboard layout pointer computation Heinrich Schuchardt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox