[PATCH v2 2/4] bootm: Add a bootm command for type IH_OS_EFI

[Cristian Ciocaltea]

On Wed, Dec 11, 2019 at 10:57:48AM +0100, Heinrich Schuchardt wrote:
> On 12/11/19 9:54 AM, Cristian Ciocaltea wrote:
> > On Tue, Dec 10, 2019 at 08:32:17PM +0100, Heinrich Schuchardt wrote:
> > > On 12/10/19 9:56 AM, Cristian Ciocaltea wrote:
> > > > Add support for booting EFI binaries contained in FIT images.
> > > > A typical usage scenario is chain-loading GRUB2 in a verified
> > > > boot environment.
> > > > 
> > > > Signed-off-by: Cristian Ciocaltea<cristian.ciocaltea at gmail.com>
> > > 
> > > Reading through the code it looks good. What I really need to do is
> > > analyze the address usage on the sandbox. To me it is unclear if
> > > images->fdt_addr is a physical address or an address in the address
> > > space of the sandbox.
> > > 
> > > Did you test this on the sandbox? You can use
> > > lib/efi_loader/helloworld.efi as a binary and the 'host load hostfs'
> > > command for loading the FIT image.
> > 
> > I only tested on qemu, I've never used the sandbox, so it's a good
> > opportunity to give it a try.
> > 
> > > Shouldn't we add booting a UEFI FIT image to the Python test in
> > > test/py/tests/test_fit.py?
> > 
> > Unfortunately I'm not familiar with the testing framework (including
> > Python scripting), but I'll do my best to add such a test.
> > 
> > > doc/uImage.FIT/signature.txt describes that several properties of the
> > > RSA public key should be stored in the control device tree.
> > > Unfortunately no example is supplied in which format they should be
> > > stored. Could you send me an example, please.
> > > 
> > > I found the following
> > > 
> > > https://github.com/bn121rajesh/ipython-notebooks/blob/master/BehindTheScene/RSAPublicKeyParamsUBoot/rsa_public_key_params_uboot.ipynb
> > > 
> > > Is this an accurate description? Or how do you get the parameters from
> > > your RSA public key?
> > 
> > My test scenario involves the following steps:
> > 
> > 1. Create a public/private key pair
> > $ openssl genpkey -algorithm RSA -out ${DEV_KEY} \
> >          -pkeyopt rsa_keygen_bits:2048 -pkeyopt rsa_keygen_pubexp:65537
> > 
> > 2. Create a certificate containing the public key
> > $ openssl req -batch -new -x509 -key ${DEV_KEY} -out ${DEV_CRT}
> > 
> > 3. Dump QEMU virt board DTB
> > $ qemu-system-arm -nographic -M virt,dumpdtb=${BOARD_DTB} \
> >          -cpu cortex-a15 -smp 1 -m 512 -bios u-boot.bin [...]
> > 
> > 4. Create (unsigned) FIT image and put the public key into DTB, with
> >     the 'required' property set, telling U-Boot that this key MUST be
> >     verified for the image to be valid
> > $ mkimage -f ${FIT_ITS} -K ${BOARD_DTB} -k ${KEYS_DIR} -r ${FIT_IMG}
> > 
> > 5. Sign the FIT image
> > $ fit_check_sign -f ${FIT_IMG} -k ${BOARD_DTB}
> > 
> > 6. Run QEMU supplying the DTB containing the public key and the
> >     u-boot binary built with CONFIG_OF_BOARD
> > $ qemu-system-arm -nographic \
> >      -M virt -cpu cortex-a15 -smp 1 -m 512 -bios u-boot.bin \
> >      -dtb ${BOARD_DTB} [...]
> > 
> > This is what I get after booting QEMU with the command above:
> > 
> > => fdt addr $fdtcontroladdr
> > => fdt print
> > / {
> >      [...]
> > 	signature {
> > 		key-dev {
> > 			required = "conf";
> > 			algo = "sha256,rsa2048";
> > 			rsa,r-squared = * 0x5ef05188 [0x00000100];
> > 			rsa,modulus = * 0x5ef05294 [0x00000100];
> > 			rsa,exponent = <0x00000000 0x00010001>;
> > 			rsa,n0-inverse = <0x649cd557>;
> > 			rsa,num-bits = <0x00000800>;
> > 			key-name-hint = "dev";
> > 		};
> > 	};
> >      [...]
> 
> See my patch
> 
> doc: fitImage: example of a signature node
> https://lists.denx.de/pipermail/u-boot/2019-December/393613.html
> 
> ---
> 
> Booting of the sandbox fails due to an incorrect address passed for the
> device tree:
> 
> => host load hostfs - $kernel_addr_r image.fit
> 26558 bytes read in 0 ms
> => bootm ${kernel_addr_r}#config-grub-fdt
> ## Loading kernel from FIT Image at 01000000 ...
>    Using 'config-grub-fdt' configuration
>    Verifying Hash Integrity ... OK
>    Trying 'helloworld' kernel subimage
>      Description:  Hello World
>      Created:      2019-12-11   9:19:22 UTC
>      Type:         Kernel Image (no loading done)
>      Compression:  uncompressed
>      Data Start:   0x010000cc
>      Data Size:    2733 Bytes = 2.7 KiB
>      Hash algo:    sha256
>      Hash value:
> 5c3ba35a1cb4abfe8a867ea6ac709574535794a7d7d03ba1ec2273b956d13983
>    Verifying Hash Integrity ... sha256+ OK
>    XIP Kernel Image (no loading done)
> ## Loading fdt from FIT Image at 01000000 ...
>    Using 'config-grub-fdt' configuration
>    Verifying Hash Integrity ... OK
>    Trying 'fdt-test' fdt subimage
>      Description:  QEMU DTB
>      Created:      2019-12-11   9:19:22 UTC
>      Type:         Flat Device Tree
>      Compression:  uncompressed
>      Data Start:   0x01000c74
>      Data Size:    19713 Bytes = 19.3 KiB
>      Architecture: ARM
>      Hash algo:    sha256
>      Hash value:
> 3e4f4e2b512f7a03a7f9ccecfb2b6bf7ceea75882370639460fd61502d903cd1
>    Verifying Hash Integrity ... sha256+ OK
>    Booting using the fdt blob at 0x1000c74
> Found 0 disks
> phys_to_virt: Cannot map sandbox address 11001c74 (SDRAM from 0 to 8000000)
> Aborted

I've checked the internal handling of the FDT images and it seems the
'ft_addr' attribute inside 'bootm_headers_t' structure points to a
mapped memory location. Since efi_install_fdt() assumes a physical
address (it calls map_sysmem() before accessing the data), it might be
enough to just reconvert 'ft_addr' via map_to_sysmem(), prior the call
to efi_install_fdt().

The issue is not present on ARM because the memory mapping utilities do
not actually perform any operations.

> Please, check both the FDT and the non-FDT case on the sandbox and
> resubmit the patch.

Yes, I'm going to (re)test on both QEMU and sandbox.

Should the upcoming patch set also include the suggested python test
or that can be added later as a separate patch?

Thanks.

> Best regards
> 
> Heinrich