diff mbox series

[v11] Boot var automatic management for removable medias

Message ID 20230920084121.1248590-1-masahisa.kojima@linaro.org
State Superseded
Headers show
Series [v11] Boot var automatic management for removable medias | expand

Commit Message

Masahisa Kojima Sept. 20, 2023, 8:41 a.m. UTC
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.

Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 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(+)


base-commit: 8bd7d08407f986fe3c69f3e198f6f7152e12dea8

Comments

Raymond Mao Sept. 20, 2023, 3:30 p.m. UTC | #1
Hi Masahisa,

Thanks for spending time on this test failure! I am not sure why this
cannot be reproduced in my end, but I totally agree with the rationale you
analyzed - my previous patch will just add boot options whenever a media is
probed.
Thanks for adding the changes into the pytest.

Regards,
Raymond
Ilias Apalodimas Sept. 26, 2023, 6:51 p.m. UTC | #2
On Wed, 20 Sept 2023 at 11:41, 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.
>
> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  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',
>                  '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',
>
> base-commit: 8bd7d08407f986fe3c69f3e198f6f7152e12dea8
> --
> 2.34.1
>

I think you should add your co-developped-by tag now since you fixed
the CI issues,

With that
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Masahisa Kojima Sept. 27, 2023, 12:42 a.m. UTC | #3
Hi Ilias,

On Wed, 27 Sept 2023 at 03:52, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> On Wed, 20 Sept 2023 at 11:41, 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.
> >
> > Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > ---
> >  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',
> >                  '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',
> >
> > base-commit: 8bd7d08407f986fe3c69f3e198f6f7152e12dea8
> > --
> > 2.34.1
> >
>
> I think you should add your co-developped-by tag now since you fixed
> the CI issues,
>
> With that
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

OK, I will resend it.

Thanks,
Masahisa Kojima
diff mbox series

Patch

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',
                 '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',