Both the Linux kernel and libbsd agree that strlcpy() should always
return strlen(src) and not include the NUL termination. The incorrect
U-Boot implementation makes it impossible to check the return value for
truncation, and breaks code written with the usual implementation in
mind (for example, fdtdec_add_reserved_memory() was subtly broken).
I reviewed all callers of strlcpy() and strlcat() and fixed them
according to my understanding of the intended function.
This reverts commit d3358ecc54 and adds
related fixes.
Fixes: d3358ecc54 ("lib: string: Fix strlcpy return value")
Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Sean Anderson <sean.anderson@seco.com>
All SPL hash algorithm options are collected in lib/Kconfig. Move
SPL_CRC32 there as well.
Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
Reviewed-by: Simon Glass <sjg@chromium.org>
Check the uuid_str_to_bin return value, skip the node
if the image-type-id property is wrong format.
Addresses-Coverity-ID: 463145 ("Error handling issues")
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
* If an error occurs in efi_disk_add_dev(), don't leak resources.
* If calloc() fails while creating the file system protocol interface,
signal an error.
* Rename efi_simple_file_system() to efi_create_simple_file_system().
* Drop a little helpful debug message.
Fixes: 2a92080d8c ("efi_loader: add file/filesys support")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Handle out of memory situation in efi_mem_carve_out().
Fixes: 5d00995c36 ("efi_loader: Implement memory allocation and map")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
On 32bit systems (pages << EFI_PAGE_SHIFT) may lead to an overflow which
does not occur in 64bit arithmetics.
An overflow of (pages << EFI_PAGE_SHIFT) on 64bit systems should be treated
as an error.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
If the hard coded array hash_algo_list[] contains an entry for an
unsupported algorithm, we should not leak resources new_efi and regs.
We should still extend the log with the digests for the supported
algorithms and not write any message.
The same holds true of tcg2_create_digest(): just continue in case
hash_algo_list[] contains an unsupported entry.
Fixes: 163a0d7e2c ("efi_loader: add PE/COFF image measurement")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Handle out of memory situation in efi_add_memory_map_pg().
Fixes: 5d00995c36 ("efi_loader: Implement memory allocation and map")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
In test_hii_database_list_package_lists() 'ret' is used for the return code
of EFI API calls and 'result' for the return value of the function. Writing
EFI_ST_FAILURE to ret is superfluous.
Fixes: 4c4fb10da2 ("efi_selftest: add HII database protocols test")
Fixes: ee3c8ba855 ("efi_selftest: fix memory allocation in HII tests")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
The efi_parse_pkcs7_header() function returns NULL on error so the check
for IS_ERR() should be changed to a NULL check.
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
While UPDATE_CAPSULE api is not fully implemented, this interface and
capsule-on-disk feature should behave in the same way, especially in
handling an empty capsule for fwu multibank, for future enhancement.
So move the guid check into efi_capsule_update_firmware().
Fixed: commit a6aafce494 ("efi_loader: use efi_update_capsule_firmware() for capsule on disk")
Reported-by: Michal Simek <michal.simek@amd.com>
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reported-by: Michal Simek <michal.simek@amd.com>
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Tested-by: Michal Simek <michal.simek@amd.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
It's pretty unlikely that anyone is going to be using EFI authentication
on a 32bit system. However, if you did, the efi_prepare_aligned_image()
function would write 8 bytes of data to the &efi_size variable and it
can only hold 4 bytes so that corrupts memory.
Fixes: 163a0d7e2c ("efi_loader: add PE/COFF image measurement")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
The EFI doesn't allow removal of handles, unless all hosted protocols
are cleanly removed. Our efi_delete_handle() is a bit intrusive.
Although it does try to delete protocols before removing a handle,
it doesn't care if that fails. Instead it only returns an error if the
handle is invalid. On top of that none of the callers of that function
check the return code.
So let's rewrite this in a way that fits the EFI spec better. Instead
of forcing the handle removal, gracefully uninstall all the handle
protocols. According to the EFI spec when the last protocol is removed
the handle will be deleted. Also switch all the callers and check the
return code. Some callers can't do anything useful apart from reporting
an error. The disk related functions on the other hand, can prevent a
medium that is being used by EFI from removal.
The only function that doesn't check the result is efi_delete_image().
But that function needs a bigger rework anyway, so we can clean it up in
the future
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Move the recursive dp_fill(dev->parent) call to a single location.
Determine uclass_id only once.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
The UEFI specification does not provide node types matching UCLASS_BLKMAP,
UCLASS_HOST, UCLASS_VIRTIO block devices.
The current implementation uses VenHw() nodes with uclass specific GUIDs
and a single byte for the device number appended. This leads to unaligned
integers in succeeding device path nodes.
The current implementation fails to create unique device paths for block
devices based on other uclasses like UCLASS_PVBLOCK.
Let's use a VenHw() node with the U-Boot GUID with a length dividable by
four and encoding blkdesc->uclass_id as well as blkdesc->devnum.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
It is not visible anywhere in Trial State if this is the first, second, etc
attempt that's why show a message to be aware about status.
Signed-off-by: Michal Simek <michal.simek@amd.com>
Acked-by: Jassi Brar <jaswinder.singh@linaro.org>
Code rewrites the last char of size with adding &. It is visible from
dfu_alt_info print before this patch:
Make dfu_alt_info: 'mtd nor0=bank0 raw 2320000 80000;bank1 raw 27a0000
8000&mtd nor0=bank0 raw 23a0000 4000000;bank1 raw 2820000 4000000'
And after it:
Make dfu_alt_info: 'mtd nor0=bank0 raw 2320000 80000;bank1 raw 27a0000
80000&mtd nor0=bank0 raw 23a0000 4000000;bank1 raw 2820000 4000000'
Size for bank0 and bank1 must be the same because it is the same image.
Signed-off-by: Michal Simek <michal.simek@amd.com>
Acked-by: Jassi Brar <jaswinder.singh@linaro.org>
Current code after capsule update (mtd write) is not changing active_index
in mdata to previous_active_index.
On the reboot this is shown but showing message
"Boot idx 1 is not matching active idx 0, changing active_idx"
which is changing active_idx and writing mdata to flash.
But when this message is visible it is not checking which state that images
are. If they have acceptance bit setup to yes everything is fine and valid
images are booted (doesn't mean the latest one).
But if acceptance bit is no and images are in trial state in_trial variable
is never setup. Which means that from new flashed image stable image can be
rewritten because in_trial is not setup properly.
Signed-off-by: Michal Simek <michal.simek@amd.com>
Acked-by: Jassi Brar <jaswinder.singh@linaro.org>
Commit 62649165cb ("lib: sparse: Make CHUNK_TYPE_RAW buffer aligned")
fixed cache alignment for systems with a D-CACHE.
However it introduced some performance regressions [1] on system
flashing huge images, such as Android.
On AM62x SK EVM, we also observe such performance penalty:
Sending sparse 'super' 1/2 (768793 KB) OKAY [ 23.954s]
Writing 'super' OKAY [ 75.926s]
Sending sparse 'super' 2/2 (629819 KB) OKAY [ 19.641s]
Writing 'super' OKAY [ 62.849s]
Finished. Total time: 182.474s
The reason for this is that we use an arbitrary small buffer
(info->blksz * 100) for transferring.
Fix it by using a bigger buffer (info->blksz * FASTBOOT_MAX_BLK_WRITE)
as suggested in the original's patch review [2].
With this patch, performance impact is mitigated:
Sending sparse 'super' 1/2 (768793 KB) OKAY [ 23.912s]
Writing 'super' OKAY [ 15.780s]
Sending sparse 'super' 2/2 (629819 KB) OKAY [ 19.581s]
Writing 'super' OKAY [ 17.192s]
Finished. Total time: 76.569s
[1] https://lore.kernel.org/r/20221118121323.4009193-1-gary.bisson@boundarydevices.com
[2] https://lore.kernel.org/r/all/43e4c17c-4483-ec8e-f843-9b4c5569bd18@seco.com/
Fixes: 62649165cb ("lib: sparse: Make CHUNK_TYPE_RAW buffer aligned")
Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
This reverts commit d927d1a808, reversing
changes made to c07ad9520c.
These changes do not pass CI currently.
Signed-off-by: Tom Rini <trini@konsulko.com>
Add MM communication support using FF-A transport
This feature allows accessing MM partitions services through
EFI MM communication protocol. MM partitions such as StandAlonneMM
or smm-gateway secure partitions which reside in secure world.
An MM shared buffer and a door bell event are used to exchange
the data.
The data is used by EFI services such as GetVariable()/SetVariable()
and copied from the communication buffer to the MM shared buffer.
The secure partition is notified about availability of data in the
MM shared buffer by an FF-A message (door bell).
On such event, MM SP can read the data and updates the MM shared
buffer with the response data.
The response data is copied back to the communication buffer and
consumed by the EFI subsystem.
MM communication protocol supports FF-A 64-bit direct messaging.
Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
Tested-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Jens Wiklander <jens.wiklander@linaro.org>
convert UUID string to little endian binary data
Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Jens Wiklander <jens.wiklander@linaro.org>
We are using our custMpk for signing that is a 4096 bit key, 4096 bit
rsa key requires a SHA512 hashing algorithm to be enabled as per the
source. Even though it is not mandated but this is how it works and is
tested.
Enables SHA512 if fit signature is enabled on K3 platforms.
Signed-off-by: Manorit Chawdhry <m-chawdhry@ti.com>
On devices with multiple USB mass storage devices errors like
Path /../USB(0x0,0x0)/USB(0x1,0x0)/Ctrl(0x0)
already installed.
are seen. This is due to creating non-unique device paths. To uniquely
identify devices we must provide path nodes for all devices on the path
from the root device.
Add support for generating device path nodes for all uclasses.
Reported-by: Suniel Mahesh <sunil@amarulasolutions.com>
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Do not assume that the preceding device path contains a single VenHW node.
Instead use the return value of dp_fill() which provides the address of the
next node.
Fixes: 23ad52fff4 ("efi_loader: device_path: support Sandbox's "host" devices")
Fixes: 19ecced71c ("efi_loader: device path for virtio block devices")
Fixes: 272ec6b453 ("efi_loader: device_path: support blkmap devices")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
The devnum value of the blk_desc structure starts from 0,
current efi_bl_create_block_device() function creates
two "efiblk#0" devices for the cases that blk_find_max_devnum()
returns -ENODEV and blk_find_max_devnum() returns 0(one device
found in this case).
This commit uses blk_next_free_devnum() instead of blk_find_max_devnum().
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
A previous patch is removing the last consumer of efi_remove_protocol().
Switch that to static and treat it as an internal API in order to force
users install and remove protocols with the appropriate EFI functions.
It's worth noting that we still have files using efi_add_protocol(). We
should convert all these to efi_install_multiple_protocol_interfaces()
and treat efi_add_protocol() in a similar manner
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
The TCG2 protocol currently adds and removes protocols with
efi_(add/remove)_protocol().
Removing protocols with efi_remove_protocol() might prove
problematic since it doesn't call DisconnectController() when
uninstalling the protocol and does not comply with the UEFI specification.
It's also beneficial for readability to have protocol installations and
removals in pairs -- IOW when efi_install_multiple_protocol_interfaces()
is called, efi_uninstall_multiple_protocol_interfaces() should be used to
remove it. So let's swap the efi_add_protocol() as well.
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
The call to efi_search_obj() is redundant as the function is called in
efi_search_protocol() too.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
U-Boot sets up the ACPI tables during startup. Rather than creating a
new set, install the existing ones. Create a memory-map record to cover
the tables.
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
The CMD_EFIDEBUG option enables debugging so it is reasonable to assume
that all effects should be made to decode the dreaded UUIDs favoured by
UEFI.
Update the table to show them all when CONFIG_CMD_EFIDEBUG is enabled.
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
We recently fixed a few issues wrt to controller handling. Add a few
test cases to cover the new code.
- return EFI_DEVICE_ERROR the first time the protocol interface of
the controller is uninstalled, after all the children have been
disconnected. This should make the drivers reconnect
- add tests to verify controllers are reconnected when uninstalling a
protocol fails
- add tests to make sure EFI_NOT_FOUND is returned if a non existent
interface is being removed
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Up to now we did not check the return value of DisconnectController.
A previous patch is fixing that taking into account what happened during
the controller disconnect. But that check takes place before our code
is trying to figure out if the interface exists to begin with. In case a
driver is not allowed to unbind -- e.g returning EFI_DEVICE_ERROR, we
will end up returning that error instead of EFI_NOT_FOUND.
Add an extra check on the top of the function to make sure the protocol
interface exists before trying to disconnect any drivers
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
efi_uninstall_protocol() calls efi_disconnect_all_drivers() but never
checks the return value. Instead it tries to identify protocols that
are still open after closing the ones that were opened with
EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL, EFI_OPEN_PROTOCOL_GET_PROTOCOL
and EFI_OPEN_PROTOCOL_TEST_PROTOCOL.
Instead of doing that, check the return value early and exit if
disconnecting the drivers failed. Also reconnect all the drivers of
a handle if protocols are still found on the handle after disconnecting
controllers and closing the remaining protocols.
While at it fix a memory leak and properly free the opened protocol
information when closing a protocol.
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
efi_disconnect_controller() doesn't reconnect drivers in case of
failure. Reconnect the disconnected drivers properly
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
The boot variables automatically generated for removable medias
should be with short form of device path without device nodes.
This is a requirement for the case that a removable media is
plugged into a different port but is still able to work with the
existing boot variables.
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>
Correct the return code for out-of-memory and no boot option found
Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Rename and move bootorder and bootoption apis from cmd to lib
for re-use between eficonfig and bootmgr
Fix 'unexpected indentation' when 'make htmldocs' after functions
are moved
Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Uncompressing a 1.7Mbytes FIT image on U-boot 2023.04 takes
approx 7s on a powerpc 8xx.
The same on U-boot 2023.07-rc6 takes approx 28s unless watchdog
is disabled.
During that decompression, LzmaDec_DecodeReal() calls schedule
1.6 million times, that is every 4µs in average.
In the past it used to be a call to WATCHDOG_RESET() which was
just calling hw_watchdog_reset().
But the combination of commit 29caf9305b ("cyclic: Use schedule()
instead of WATCHDOG_RESET()") and commit 26e8ebcd7c ("watchdog:
mpc8xxx: Make it generic") results in an heavier processing.
However, there is absolutely no point in calling schedule() that
often.
By moving and keeping only one call to schedule() in the main
loop the number of calls is reduced to 1.2 million which is still
too much. So add logic to only call schedule every 1024 times.
That leads to a call to schedule approx every 6ms which is still
far enough to entertain the watchdog which has a 1s timeout on
powerpc 8xx.
powerpc 8xx being one of the slowest targets we have today in
U-boot, and most other watchdogs having a timeout of one minutes
instead of one second like the 8xx, this fix should not have
negative impact on other targets.
Fixes: 29caf9305b ("cyclic: Use schedule() instead of WATCHDOG_RESET()")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Simon Glass <sjg@chromium.org>
This fixes CVE-2022-37434 [1] and bases on 2 commits from Mark
Adler's zlib master repo - the original fix of CVE bug [2] and
the fix for the fix [3].
[1]
https://github.com/advisories/GHSA-cfmr-vrgj-vqwv
[2]
eff308af42
[3]
1eb7682f84
Fixes: e89516f031 ("zlib: split up to match original source tree")
Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
At present livetree can only be used for the control FDT. It is useful
to be able to use the ofnode API for other FDTs, e.g. those used by
the upcoming configuration editor.
We already have most of the support present, and tests can be marked with
the UT_TESTF_OTHER_FDT flag to use another FDT as a special case. But
with this change, the functionality becomes more generally available.
Plumb in the require support.
Signed-off-by: Simon Glass <sjg@chromium.org>
Ensure that the block of memory used by live tree is aligned according to
the default for structures. This ensures that the root node appears at
the start of the block, so it can be used with free(), rather than being
4 bytes later in some cases.
This corrects a rather obscure bug in unflatten_device_tree().
Fixes: 8b50d526ea ("dm: Add a function to create a 'live' device tree")
Signed-off-by: Simon Glass <sjg@chromium.org>
Debian's arm64 UEFI Secure Boot shim makes the EFI variable store run
out of space while mirroring its MOK database to variables. This can be
observed in QEMU like so:
$ tools/buildman/buildman -o build/qemu_arm64 --boards=qemu_arm64 -w
$ cd build/qemu_arm64
$ curl -L -o debian.iso \
https://cdimage.debian.org/debian-cd/current/arm64/iso-cd/debian-12.0.0-arm64-netinst.iso
$ qemu-system-aarch64 \
-nographic -bios u-boot.bin \
-machine virt -cpu cortex-a53 -m 1G -smp 2 \
-drive if=virtio,file=debian.iso,index=0,format=raw,readonly=on,media=cdrom
[...]
=> # interrupt autoboot
=> env set -e -bs -nv -rt -guid 605dab50-e046-4300-abb6-3dd810dd8b23 SHIM_VERBOSE 1
=> boot
[...]
mok.c:296:mirror_one_esl() SetVariable("MokListXRT43", ... varsz=0x4C) = Out of Resources
mok.c:452:mirror_mok_db() esd:0x7DB92D20 adj:0x30
Failed to set MokListXRT: Out of Resources
mok.c:767:mirror_one_mok_variable() mirror_mok_db("MokListXRT", datasz=17328) returned Out of Resources
mok.c:812:mirror_one_mok_variable() returning Out of Resources
Could not create MokListXRT: Out of Resources
[...]
Welcome to GRUB!
This would normally be fine as shim would continue to run grubaa64.efi,
but shim's error handling code for this case has a bug [1] that causes a
synchronous abort on at least chromebook_kevin (but apparently not on
QEMU arm64).
Double the default variable store size so the variables fit. There is a
note about this value matching PcdFlashNvStorageVariableSize when
EFI_MM_COMM_TEE is enabled, so keep the old default in that case.
[1] https://github.com/rhboot/shim/pull/577
Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
The efi_var_mem_free() function calculates the available size for a new
EFI variable by subtracting the occupied buffer size and the overhead
for a new variable from the maximum buffer size set in Kconfig. This
is then returned as QueryVariableInfo()'s RemainingVariableStorageSize
output.
This can underflow as the calculation is done in and processed as
unsigned integer types. Check for underflow before doing the subtraction
and return zero if there's no space.
Fixes: f1f990a8c9 ("efi_loader: memory buffer for variables")
Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
