[PATCH v8 7/8] test: efi: boot: Set up an image suitable for EFI testing
Heinrich Schuchardt
xypron.glpk at gmx.de
Mon Oct 28 20:33:04 CET 2024
On 10/28/24 18:02, Simon Glass wrote:
> Hi Heinrich,
>
> On Mon, 28 Oct 2024 at 11:25, Heinrich Schuchardt <xypron.glpk at gmx.de
> <mailto:xypron.glpk at gmx.de>> wrote:
> >
> > On 10/28/24 07:08, Heinrich Schuchardt wrote:
> > > On 10/22/24 14:00, Simon Glass wrote:
> > >> Create a new disk for use with tests, which contains the new 'testapp'
> > >> EFI app specifically intended for testing the EFI loader.
> > >>
> > >> Attach it to the USB device, since most testing is currently done with
> > >> mmc.
> > >>
> > >> Initially this image will be used to test the EFI bootmeth.
> > >>
> > >> Fix a stale comment in prep_mmc_bootdev() while we are here.
> > >>
> > >> For now this uses sudo and a compressed fallback file, like all the
> > >> other bootstd tests. Once this series is in, the patch which moves
> > >> this to use user-space tools will be cleaned up and re-submitted.
> > >>
> > >> Signed-off-by: Simon Glass <sjg at chromium.org
> <mailto:sjg at chromium.org>>
> > >>
> > >> ---
> > >> Here is the patch to avoid sudo and CI fallback:
> > >>
> > >> [1] https://patchwork.ozlabs.org/project/uboot/patch/ <https://
> patchwork.ozlabs.org/project/uboot/patch/>
> > >> 20240802093322.15240-1-richard at nod.at/
> <http://20240802093322.15240-1-richard@nod.at/>
> > >>
> > >> (no changes since v1)
> > >>
> > >> arch/sandbox/dts/test.dts | 2 +-
> > >> test/boot/bootdev.c | 18 +++++++++-
> > >> test/boot/bootflow.c | 2 +-
> > >> test/py/tests/bootstd/flash1.img.xz | Bin 0 -> 5016 bytes
> > >> test/py/tests/test_ut.py | 52 ++++++++++++++++++++++
> ++----
> > >> 5 files changed, 65 insertions(+), 9 deletions(-)
> > >> create mode 100644 test/py/tests/bootstd/flash1.img.xz
> > >>
> > >> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> > >> index 9bf44ae3b0b..f40fd5a2b16 100644
> > >> --- a/arch/sandbox/dts/test.dts
> > >> +++ b/arch/sandbox/dts/test.dts
> > >> @@ -1512,7 +1512,7 @@
> > >> flash-stick at 1 {
> > >> reg = <1>;
> > >> compatible = "sandbox,usb-flash";
> > >> - sandbox,filepath = "testflash1.bin";
> > >> + sandbox,filepath = "flash1.img";
> > >> };
> > >>
> > >> flash-stick at 2 {
> > >> diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c
> > >> index 369105ca4cf..c892854b227 100644
> > >> --- a/test/boot/bootdev.c
> > >> +++ b/test/boot/bootdev.c
> > >> @@ -221,6 +221,10 @@ static int bootdev_test_order(struct
> > >> unit_test_state *uts)
> > >> /* Use the environment variable to override it */
> > >> ut_assertok(env_set("boot_targets", "mmc1 mmc2 usb"));
> > >> ut_assertok(bootflow_scan_first(NULL, NULL, &iter, 0, &bflow));
> > >> +
> > >> + /* get the usb device which has a backing file (flash1.img) */
> > >> + ut_asserteq(0, bootflow_scan_next(&iter, &bflow));
> > >> +
> > >> ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
> > >> ut_asserteq(5, iter.num_devs);
> > >> ut_asserteq_str("mmc1.bootdev", iter.dev_used[0]->name);
> > >> @@ -260,7 +264,11 @@ static int bootdev_test_order(struct
> > >> unit_test_state *uts)
> > >> ut_assertok(bootflow_scan_first(NULL, NULL, &iter, 0, &bflow));
> > >> ut_asserteq(2, iter.num_devs);
> > >>
> > >> - /* Now scan past mmc1 and make sure that the 3 USB devices show
> > >> up */
> > >> + /*
> > >> + * Now scan past mmc1 and make sure that the 3 USB devices show
> > >> up. The
> > >> + * first one has a backing file so returns success
> > >> + */
> > >> + ut_asserteq(0, bootflow_scan_next(&iter, &bflow));
> > >> ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
> > >> ut_asserteq(6, iter.num_devs);
> > >> ut_asserteq_str("mmc2.bootdev", iter.dev_used[0]->name);
> > >> @@ -322,6 +330,10 @@ static int bootdev_test_prio(struct
> > >> unit_test_state *uts)
> > >>
> > >> /* 3 MMC and 3 USB bootdevs: MMC should come before USB */
> > >> ut_assertok(bootflow_scan_first(NULL, NULL, &iter, 0, &bflow));
> > >> +
> > >> + /* get the usb device which has a backing file (flash1.img) */
> > >> + ut_asserteq(0, bootflow_scan_next(&iter, &bflow));
> > >> +
> > >> ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
> > >> ut_asserteq(6, iter.num_devs);
> > >> ut_asserteq_str("mmc2.bootdev", iter.dev_used[0]->name);
> > >> @@ -339,6 +351,10 @@ static int bootdev_test_prio(struct
> > >> unit_test_state *uts)
> > >> bootflow_iter_uninit(&iter);
> > >> ut_assertok(bootflow_scan_first(NULL, NULL, &iter,
> BOOTFLOWIF_HUNT,
> > >> &bflow));
> > >> +
> > >> + /* get the usb device which has a backing file (flash1.img) */
> > >> + ut_asserteq(0, bootflow_scan_next(&iter, &bflow));
> > >> +
> > >> ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
> > >> ut_asserteq(7, iter.num_devs);
> > >> ut_asserteq_str("usb_mass_storage.lun0.bootdev",
> > >> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> > >> index 539abe63ebe..d94c07963d5 100644
> > >> --- a/test/boot/bootflow.c
> > >> +++ b/test/boot/bootflow.c
> > >> @@ -533,7 +533,7 @@ static int prep_mmc_bootdev(struct unit_test_state
> > >> *uts, const char *mmc_dev,
> > >>
> > >> order[2] = mmc_dev;
> > >>
> > >> - /* Enable the mmc4 node since we need a second bootflow */
> > >> + /* Enable the requested mmc node since we need a second
> bootflow */
> > >> root = oftree_root(oftree_default());
> > >> node = ofnode_find_subnode(root, mmc_dev);
> > >> ut_assert(ofnode_valid(node));
> > >> diff --git a/test/py/tests/bootstd/flash1.img.xz b/test/py/tests/
> > >> bootstd/flash1.img.xz
> > >> new file mode 100644
> > >> index
> > >>
> 0000000000000000000000000000000000000000..9148b91a20c1f0acb54449b124d1408961e92507
> > >
> > > We should not accept such binaries. This was just how xz was hacked
> > > (https://en.wikipedia.org/wiki/XZ_Utils_backdoor <https://
> en.wikipedia.org/wiki/XZ_Utils_backdoor>).
> > >
> > > This binary wouldn't conform to the GPL 2.0 license either.
>
> Yes, I've offered to tidy up the patch and drop this binary (and the
> others) once this test lands.
>
> [..]
>
> > >> +def setup_efi_image(cons):
> > >> + """Create a 20MB disk image with an EFI app on it"""
> > >> + devnum = 1
> > >> + basename = 'flash'
> > >> + fname, mnt = setup_image(cons, devnum, 0xc, second_part=True,
> > >> + basename=basename)
> > >> +
> > >> + loop = None
> > >> + mounted = False
> > >> + complete = False
> > >> + try:
> > >> + loop = mount_image(cons, fname, mnt, 'ext4')
> > >> + mounted = True
> > >> + efi_dir = os.path.join(mnt, 'EFI')
> > >> + mkdir_cond(efi_dir)
> > >> + bootdir = os.path.join(efi_dir, 'BOOT')
> > >> + mkdir_cond(bootdir)
> > >> + efi_src = os.path.join(cons.config.build_dir,
> > >> + f'lib/efi_loader/testapp.efi')
> > >> + efi_dst = os.path.join(bootdir, 'BOOTSBOX.EFI')
> >
> > As said I can live with the -n switch though that standards defying
> > bootsbox.efi name provides no benefit whatsoever. But, please, do not
> > ignore the switch.
>
> The -N flag is provided for allowing sandbox to use the correct, default
> name for the underlying host. It is not intended for use in CI, where
> 'sandbox is sandbox'. So far, CI calls sandbox without that flag, so the
> code above is actually correct.
>
> Regards,
> Simon
Hello Simon,
We are in round 8 of this patch series and it is still broken:
You supply an xz image which we should never accept for security reasons.
That xz image contains an EFI/BOOT/BOOTSBOX.EFI blob. This does not
comply to the GPLv2 license.
That blob is an amd64 binary. So nobody will be able it to run it in the
sandbox built on the riscv64 or arm64 machines.
* Please, remove the blob.
* Do the same for all blobs in test/py/tests/bootstd/.
* Provide a test test runs on all EFI architectures.
* Make sure that we can run the test both on QEMU and on real hardware.
* Please, comply in your test to the UEFI specification.
Best regards
Heinrich
More information about the U-Boot
mailing list