Hi Tom, Heinrich, Ilias, # Added Tom since this patch is delegated to him
On Wed, 27 Sept 2023 at 10:06, Masahisa Kojima masahisa.kojima@linaro.org wrote:
From: Raymond Mao raymond.mao@linaro.org
Changes for complying to EFI spec ยง3.5.1.1 'Removable Media Boot Behavior'. Boot variables can be automatically generated during a removable media is probed. At the same time, unused boot variables will be detected and removed.
Please note that currently the function 'efi_disk_remove' has no ability to distinguish below two scenarios a) Unplugging of a removable media under U-Boot b) U-Boot exiting and booting an OS Thus currently the boot variables management is not added into 'efi_disk_remove' to avoid boot options being added/erased repeatedly under scenario b) during power cycles See TODO comments under function 'efi_disk_remove' for more details
The original efi_secboot tests expect that BootOrder EFI variable is not defined. With this commit, the BootOrder EFI variable is automatically added when the disk is detected. The original efi_secboot tests end up with unexpected failure. The efi_secboot tests need to be modified to clear the BootOrder EFI variable at the beginning of each test.
Co-developed-by: Masahisa Kojima masahisa.kojima@linaro.org Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org Signed-off-by: Raymond Mao raymond.mao@linaro.org Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
lib/efi_loader/efi_disk.c | 18 ++++++++++++++++++ lib/efi_loader/efi_setup.c | 7 +++++++ test/py/tests/test_efi_secboot/test_signed.py | 9 +++++++++ .../test_efi_secboot/test_signed_intca.py | 3 +++ .../py/tests/test_efi_secboot/test_unsigned.py | 3 +++ 5 files changed, 40 insertions(+)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index f0d76113b0..b808a7fe62 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -690,6 +690,13 @@ int efi_disk_probe(void *ctx, struct event *event) return -1; }
/* only do the boot option management when UEFI sub-system is initialized */if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) && efi_obj_list_initialized == EFI_SUCCESS) {ret = efi_bootmgr_update_media_device_boot_option();if (ret != EFI_SUCCESS)return -1;}return 0;}
@@ -742,6 +749,17 @@ int efi_disk_remove(void *ctx, struct event *event) dev_tag_del(dev, DM_TAG_EFI);
return 0;
/** TODO A flag to distinguish below 2 different scenarios of this* function call is needed:* a) Unplugging of a removable media under U-Boot* b) U-Boot exiting and booting an OS* In case of scenario a), efi_bootmgr_update_media_device_boot_option()* needs to be invoked here to update the boot options and remove the* unnecessary ones.*/}
/** diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index 58d4e13402..69c8b27730 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -245,6 +245,13 @@ efi_status_t efi_init_obj_list(void) if (ret != EFI_SUCCESS) goto out;
if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {/* update boot option after variable service initialized */ret = efi_bootmgr_update_media_device_boot_option();if (ret != EFI_SUCCESS)goto out;}/* Define supported languages */ ret = efi_init_platform_lang(); if (ret != EFI_SUCCESS)diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py index ca52e853d8..b77b60e223 100644 --- a/test/py/tests/test_efi_secboot/test_signed.py +++ b/test/py/tests/test_efi_secboot/test_signed.py @@ -28,6 +28,7 @@ class TestEfiSignedImage(object): # Test Case 1a, run signed image if no PK output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder',
This patch itself is trying to automatically add the EFI boot option(BOOT####) and update the BootOrder variable. I now understand that clearing the BootOrder variable is not an appropriate fix in this efi_secboot test, 'setenv -e -nv -bs -rt BootOrder' fails when the BootOrder variable is not found. When booting the system with BootNext fails, efibootmgr falls back to BootOrder. Instead of loading an image with BootNext, efi_secboot tests need to explicitly set the BootOrder variable.
I will revise this patch, and let me include this patch in EFI HTTP Boot series[1]. As discussed with Ilias, EFI HTTP Boot series[1] will rely heavily on this patch.
[1] https://lore.kernel.org/u-boot/20230927093631.1595494-1-masahisa.kojima@lina...
Thanks, Masahisa Kojima
'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed -s ""', 'efidebug boot next 1', 'bootefi bootmgr'])@@ -52,6 +53,7 @@ class TestEfiSignedImage(object): # Test Case 2a, db is not yet installed output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'fatload host 0:1 4000000 KEK.auth', 'setenv -e -nv -bs -rt -at -i 4000000:$filesize KEK', 'fatload host 0:1 4000000 PK.auth',@@ -96,6 +98,7 @@ class TestEfiSignedImage(object): # Test Case 3a, rejected by dbx output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000:$filesize dbx', 'fatload host 0:1 4000000 KEK.auth',@@ -132,6 +135,7 @@ class TestEfiSignedImage(object): # Test Case 4, rejected by dbx output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'fatload host 0:1 4000000 dbx_hash.auth', 'setenv -e -nv -bs -rt -at -i 4000000:$filesize dbx', 'fatload host 0:1 4000000 db.auth',@@ -161,6 +165,7 @@ class TestEfiSignedImage(object): # is verified output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000:$filesize db', 'fatload host 0:1 4000000 KEK.auth',@@ -217,6 +222,7 @@ class TestEfiSignedImage(object): # is verified. Same as before but reject dbx_hash1.auth only output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000:$filesize db', 'fatload host 0:1 4000000 KEK.auth',@@ -294,6 +300,7 @@ class TestEfiSignedImage(object): with u_boot_console.log.section('Test Case 7a'): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000:$filesize db', 'fatload host 0:1 4000000 KEK.auth',@@ -317,6 +324,7 @@ class TestEfiSignedImage(object): with u_boot_console.log.section('Test Case 7b'): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000:$filesize db', 'fatload host 0:1 4000000 KEK.auth',@@ -348,6 +356,7 @@ class TestEfiSignedImage(object): # Test Case 8a, Secure boot is not yet forced output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld_forged.efi.signed -s ""', 'efidebug boot next 1', 'efidebug test bootmgr'])diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py index d8d599d22f..318715fa08 100644 --- a/test/py/tests/test_efi_secboot/test_signed_intca.py +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py @@ -30,6 +30,7 @@ class TestEfiSignedImageIntca(object): # Test Case 1a, with no Int CA and not authenticated by root CA output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'fatload host 0:1 4000000 db_c.auth', 'setenv -e -nv -bs -rt -at -i 4000000:$filesize db', 'fatload host 0:1 4000000 KEK.auth',@@ -63,6 +64,7 @@ class TestEfiSignedImageIntca(object): # Test Case 2a, unsigned and not authenticated by root CA output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'fatload host 0:1 4000000 KEK.auth', 'setenv -e -nv -bs -rt -at -i 4000000:$filesize KEK', 'fatload host 0:1 4000000 PK.auth',@@ -105,6 +107,7 @@ class TestEfiSignedImageIntca(object): # Test Case 3a, revoked by int CA in dbx output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'fatload host 0:1 4000000 dbx_b.auth', 'setenv -e -nv -bs -rt -at -i 4000000:$filesize dbx', 'fatload host 0:1 4000000 db_c.auth',diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py index df63f0df08..9042a46ccc 100644 --- a/test/py/tests/test_efi_secboot/test_unsigned.py +++ b/test/py/tests/test_efi_secboot/test_unsigned.py @@ -28,6 +28,7 @@ class TestEfiUnsignedImage(object): # Test Case 1 output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'fatload host 0:1 4000000 KEK.auth', 'setenv -e -nv -bs -rt -at -i 4000000:$filesize KEK', 'fatload host 0:1 4000000 PK.auth',@@ -55,6 +56,7 @@ class TestEfiUnsignedImage(object): # Test Case 2 output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'fatload host 0:1 4000000 db_hello.auth', 'setenv -e -nv -bs -rt -at -i 4000000:$filesize db', 'fatload host 0:1 4000000 KEK.auth',@@ -79,6 +81,7 @@ class TestEfiUnsignedImage(object): # Test Case 3a, rejected by dbx output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img,
'setenv -e -nv -bs -rt BootOrder', 'fatload host 0:1 4000000 db_hello.auth', 'setenv -e -nv -bs -rt -at -i 4000000:$filesize dbx', 'fatload host 0:1 4000000 KEK.auth',-- 2.34.1