On 18.10.23 08:56, Felix Moessbauer wrote:
> This patch implements the LoaderDevicePartUUID part of the systemd boot
> loader interface to pass data from the loader to the OS / systemd. The
> data is passed via EFI variables which are set by the first-stage loader
> (the one on the ESP), or alternatively by the first loader that is
> executed. By that, userspace components can later inspect this variable
> to e.g. limit the search for config partitions to the device it was
> bootet from. Currently only the LoaderDevicePartUUID is implemented.
>
> Technically, the loader asks the EFI API for the UUID of the partition it
> is executed from. Normally that is the ESP partition. Then, this UUID is
> assigned to the LoaderDevicePartUUID EFI variable (in case not set yet).
> This logic is crucial to correctly support chain-loading uses-cases and
> also aligned with how systemd boot implements this.
>
> For the sake of completeness, this logic is also added to the efi stub.
> When using it in combination with the EBG loader, this is irrelevant,
> but when starting the UKI directly it is needed.
>
> Signed-off-by: Felix Moessbauer <felix.moessba...@siemens.com>
> ---
> Makefile.am | 2 ++
> include/loader_interface.h | 25 ++++++++++++++
> kernel-stub/main.c | 11 ++++++
> loader_interface.c | 69 ++++++++++++++++++++++++++++++++++++++
> main.c | 12 +++++++
> 5 files changed, 119 insertions(+)
> create mode 100644 include/loader_interface.h
> create mode 100644 loader_interface.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 02e9cac..64d94ef 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -185,12 +185,14 @@ efi_sources = \
> env/syspart.c \
> env/fatvars.c \
> utils.c \
> + loader_interface.c \
> bootguard.c \
> main.c
>
> kernel_stub_name = kernel-stub$(MACHINE_TYPE_NAME).efi
>
> kernel_stub_sources = \
> + loader_interface.c \
> kernel-stub/fdt.c \
> kernel-stub/initrd.c \
> kernel-stub/main.c
> diff --git a/include/loader_interface.h b/include/loader_interface.h
> new file mode 100644
> index 0000000..40f0167
> --- /dev/null
> +++ b/include/loader_interface.h
> @@ -0,0 +1,25 @@
> +/*
> + * EFI Boot Guard
> + *
> + * Copyright (c) Siemens AG, 2023
> + *
> + * Authors:
> + * Felix Moessbauer <felix.moessba...@siemens.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#pragma once
> +
> +typedef struct _BG_INTERFACE_PARAMS {
> + CHAR16 *loader_device_part_uuid;
> +} BG_INTERFACE_PARAMS;
> +
> +// systemd bootloader interface vendor id
> +extern EFI_GUID vendor_guid;
> +
> +EFI_STATUS set_bg_interface_vars(const BG_INTERFACE_PARAMS *params);
> +CHAR16 *disk_get_part_uuid(EFI_HANDLE *handle);
> diff --git a/kernel-stub/main.c b/kernel-stub/main.c
> index 55873e5..9e5feec 100644
> --- a/kernel-stub/main.c
> +++ b/kernel-stub/main.c
> @@ -17,6 +17,7 @@
>
> #include "kernel-stub.h"
> #include "version.h"
> +#include "loader_interface.h"
>
> typedef struct {
> UINT8 Ignore[60];
> @@ -113,6 +114,7 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle,
> EFI_SYSTEM_TABLE *system_table)
> const SECTION *section;
> EFI_STATUS status, cleanup_status;
> UINTN n, kernel_pages;
> + BG_INTERFACE_PARAMS bg_interface_params;
>
> this_image = image_handle;
> InitializeLib(image_handle, system_table);
> @@ -230,6 +232,15 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle,
> EFI_SYSTEM_TABLE *system_table)
> info(L"Using firmware-provided device tree");
> }
>
> + UINT16 *boot_medium_uuidstr =
> + disk_get_part_uuid(stub_image->DeviceHandle);
> + bg_interface_params.loader_device_part_uuid = boot_medium_uuidstr;
> + status = set_bg_interface_vars(&bg_interface_params);
> + if (EFI_ERROR(status)) {
> + error(L"could not set interface vars", status);
> + }
> + FreePool(boot_medium_uuidstr);
> +
> kernel_entry = (EFI_IMAGE_ENTRY_POINT)
> ((UINT8 *) kernel_image.ImageBase +
> pe_header->Opt.AddressOfEntryPoint);
> diff --git a/loader_interface.c b/loader_interface.c
> new file mode 100644
> index 0000000..abd9ca5
> --- /dev/null
> +++ b/loader_interface.c
> @@ -0,0 +1,69 @@
> +/*
> + * EFI Boot Guard
> + *
> + * Copyright (c) Siemens AG, 2023
> + *
> + * Authors:
> + * Felix Moessbauer <felix.moessba...@siemens.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#include <efi.h>
> +#include <efilib.h>
> +#include "loader_interface.h"
> +#include "utils.h"
> +
> +EFI_GUID vendor_guid = {0x4a67b082,
> + 0x0a4c,
> + 0x41cf,
> + {0xb6, 0xc7, 0x44, 0x0b, 0x29, 0xbb, 0x8c, 0x4f}};
> +
> +EFI_STATUS set_bg_interface_vars(const BG_INTERFACE_PARAMS *params)
> +{
> + EFI_STATUS status = EFI_SUCCESS;
> + UINT32 attribs =
> + EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS;
> +
> + // only set this if not set by a previous stage loader
> + UINTN readsize = 0;
> + if (RT->GetVariable(L"LoaderDevicePartUUID", &vendor_guid, NULL,
> + &readsize, NULL) == EFI_NOT_FOUND) {
> + status = RT->SetVariable(
> + L"LoaderDevicePartUUID", &vendor_guid, attribs,
> + StrLen(params->loader_device_part_uuid) *
> + sizeof(UINT16),
> + params->loader_device_part_uuid);
> + }
> + return status;
> +}
> +
> +CHAR16 *disk_get_part_uuid(EFI_HANDLE *handle)
> +{
> + EFI_STATUS err;
> + EFI_DEVICE_PATH *dp;
> + err = BS->HandleProtocol(handle, &DevicePathProtocol, (void **)&dp);
> + if (EFI_ERROR(err)) {
> + return NULL;
> + }
> +
> + for (; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
> + if (dp->Type != MEDIA_DEVICE_PATH ||
> + dp->SubType != MEDIA_HARDDRIVE_DP) {
> + continue;
> + }
> +
> + HARDDRIVE_DEVICE_PATH *hd = (HARDDRIVE_DEVICE_PATH *)dp;
> + if (hd->SignatureType != SIGNATURE_TYPE_GUID) {
> + continue;
> + }
> + UINT16 *buffer = AllocatePool(sizeof(UINT16) * 37);
> + GuidToString(buffer, (EFI_GUID *)hd->Signature);
> + return buffer;
> + }
> +
> + return NULL;
> +}
> diff --git a/main.c b/main.c
> index 8fa3ab3..0ff121a 100644
> --- a/main.c
> +++ b/main.c
> @@ -22,6 +22,7 @@
> #include <configuration.h>
> #include "version.h"
> #include "utils.h"
> +#include "loader_interface.h"
>
> extern const unsigned long init_array_start[];
> extern const unsigned long init_array_end[];
> @@ -113,6 +114,7 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle,
> EFI_SYSTEM_TABLE *system_table)
> EFI_STATUS status;
> BG_STATUS bg_status;
> BG_LOADER_PARAMS bg_loader_params;
> + BG_INTERFACE_PARAMS bg_interface_params;
> CHAR16 *tmp;
>
> ZeroMem(&bg_loader_params, sizeof(bg_loader_params));
> @@ -184,6 +186,16 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle,
> EFI_SYSTEM_TABLE *system_table)
> error_exit(L"Cannot load specified kernel image", status);
> }
>
> + UINT16 *boot_medium_uuidstr =
> + disk_get_part_uuid(loaded_image->DeviceHandle);
> + bg_interface_params.loader_device_part_uuid = boot_medium_uuidstr;
> + status = set_bg_interface_vars(&bg_interface_params);
> + if (EFI_ERROR(status)) {
> + error_exit(L"Cannot set bootloader interface variables",
> + status);
> + }
It turned out that this implementation can cause problems in the field.
Comparing it to systemd-boot
(https://github.com/systemd/systemd/blob/main/src/boot/export-vars.c),
we have at least two major differences
- if setting the variable fails (seen in real life), we bail out while
systemd ignores the error
- if disk_get_part_uuid fails, we will crash over a NULL pointer
We need to fix both.
Jan
> + INFO(L"LoaderDevicePartUUID=%s\n", boot_medium_uuidstr);
> + FreePool(boot_medium_uuidstr);
> FreePool(payload_dev_path);
> FreePool(boot_medium_path);
>
--
Siemens AG, Foundational Technologies
Linux Expert Center
--
You received this message because you are subscribed to the Google Groups "EFI
Boot Guard" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to efibootguard-dev+unsubscr...@googlegroups.com.
To view this discussion visit
https://groups.google.com/d/msgid/efibootguard-dev/747631f5-eacc-42b6-97ca-08e7bfb42365%40siemens.com.
