[PATCH 0/2] efi: small hii set_keyboard_layout conformance improvement

Hi,
The following couple of patches make UEFI HII set_keyboard_layout() more conforming in the case of invalid input parameter and add a selftest. This is sent in this order to avoid breaking `bootefi selftest' in the middle but feel free to apply in any order if preferred.
Best regards, Vincent.
Vincent Stehlé (2): efi_loader: refine set_keyboard_layout() status efi_selftest: add hii set keyboard layout test case
lib/efi_loader/efi_hii.c | 3 +++ lib/efi_selftest/efi_selftest_hii.c | 12 ++++++++++++ 2 files changed, 15 insertions(+)

As per the EFI specification, the HII database protocol function set_keyboard_layout() must return EFI_INVALID_PARAMETER when it is called with a NULL key_guid argument. Modify the function accordingly to improve conformance.
Signed-off-by: Vincent Stehlé vincent.stehle@arm.com Cc: Heinrich Schuchardt xypron.glpk@gmx.de Cc: Ilias Apalodimas ilias.apalodimas@linaro.org --- lib/efi_loader/efi_hii.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c index 27db3be6a17..3b54ecb11ac 100644 --- a/lib/efi_loader/efi_hii.c +++ b/lib/efi_loader/efi_hii.c @@ -758,6 +758,9 @@ set_keyboard_layout(const struct efi_hii_database_protocol *this, { EFI_ENTRY("%p, %pUs", this, key_guid);
+ if (!key_guid) + return EFI_EXIT(EFI_INVALID_PARAMETER); + return EFI_EXIT(EFI_NOT_FOUND); }

On 1/6/23 10:46, Vincent Stehlé wrote:
As per the EFI specification, the HII database protocol function set_keyboard_layout() must return EFI_INVALID_PARAMETER when it is called with a NULL key_guid argument. Modify the function accordingly to improve conformance.
Signed-off-by: Vincent Stehlé vincent.stehle@arm.com Cc: Heinrich Schuchardt xypron.glpk@gmx.de Cc: Ilias Apalodimas ilias.apalodimas@linaro.org
lib/efi_loader/efi_hii.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c index 27db3be6a17..3b54ecb11ac 100644 --- a/lib/efi_loader/efi_hii.c +++ b/lib/efi_loader/efi_hii.c @@ -758,6 +758,9 @@ set_keyboard_layout(const struct efi_hii_database_protocol *this, { EFI_ENTRY("%p, %pUs", this, key_guid);
- if (!key_guid)
return EFI_EXIT(EFI_INVALID_PARAMETER);
This is just suppressing an SCT warning for an unimplemented function. I think we should complete the implementation of the HII protocols instead of trying to hide the deficiency.
Best regards
Heinrich
- return EFI_EXIT(EFI_NOT_FOUND); }

On Fri, 6 Jan 2023 at 17:27, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 1/6/23 10:46, Vincent Stehlé wrote:
As per the EFI specification, the HII database protocol function set_keyboard_layout() must return EFI_INVALID_PARAMETER when it is called with a NULL key_guid argument. Modify the function accordingly to improve conformance.
Signed-off-by: Vincent Stehlé vincent.stehle@arm.com Cc: Heinrich Schuchardt xypron.glpk@gmx.de Cc: Ilias Apalodimas ilias.apalodimas@linaro.org
lib/efi_loader/efi_hii.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c index 27db3be6a17..3b54ecb11ac 100644 --- a/lib/efi_loader/efi_hii.c +++ b/lib/efi_loader/efi_hii.c @@ -758,6 +758,9 @@ set_keyboard_layout(const struct efi_hii_database_protocol *this, { EFI_ENTRY("%p, %pUs", this, key_guid);
if (!key_guid)
return EFI_EXIT(EFI_INVALID_PARAMETER);
This is just suppressing an SCT warning for an unimplemented function. I think we should complete the implementation of the HII protocols instead of trying to hide the deficiency.
I don't think we are hiding the fact that the function isn't implemented here. 34.8.11 EFI_HII_DATABASE_PROTOCOL.SetKeyboardLayout() says that we must return if the KeyGuid is NULL
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Best regards
Heinrich
}return EFI_EXIT(EFI_NOT_FOUND);

Add a test for the case when the HII database protocol set_keyboard_layout() function is called with a NULL key_guid argument.
Signed-off-by: Vincent Stehlé vincent.stehle@arm.com Cc: Heinrich Schuchardt xypron.glpk@gmx.de Cc: Ilias Apalodimas ilias.apalodimas@linaro.org --- lib/efi_selftest/efi_selftest_hii.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_hii.c b/lib/efi_selftest/efi_selftest_hii.c index eaf3b0995d4..f4b55889e29 100644 --- a/lib/efi_selftest/efi_selftest_hii.c +++ b/lib/efi_selftest/efi_selftest_hii.c @@ -564,7 +564,19 @@ out: */ static int test_hii_database_set_keyboard_layout(void) { + efi_status_t ret; + PRINT_TESTNAME; + + /* Invalid key guid. */ + ret = hii_database_protocol->set_keyboard_layout( + hii_database_protocol, NULL); + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("set_keyboard_layout returned %u not invalid\n", + (unsigned int)ret); + return EFI_ST_FAILURE; + } + /* set_keyboard_layout() not implemented yet */ return EFI_ST_SUCCESS; }
participants (3)
-
Heinrich Schuchardt
-
Ilias Apalodimas
-
Vincent Stehlé