[PATCH 0/3] Limit config probing to current block device

[Felix Moessbauer]

Dear Devs,

this series changes how configuration environments are located.
By that, accidential collisions with environments from other media
(e.g. USB drives) can be avoided. For the recovery use-case, we
provide an option to override this. In addition, this is a first step
towards integrating with the systemd boot loader interface.

The core idea is to forward the boot device information from the
bootloader to the userspace libraries by using EFI variables. This
is required, as properly locating the block device of the config
partitions is tricky, just based on the device the rootfs is on.
Consider the following situations:

- rootfs on different disk
- rootfs on device mapper or mdadm

As we want to stick to the systemd boot loader interface, we do
not forward the UUIDs of the config envs, but just of the partition
the loader was started from (this information is provided by EFI).
Usually this is the UUID of the ESP. In userspace, we then resolve the
backing / parent device of the ESP. This is trivial, as both partitions
need to reside on the same device. Once we have that information, we
can limit the probing to that device.

Best regards,
Felix Moessbauer
Siemens AG

Felix Moessbauer (3):
efi: implement systemd boot loader interface
libebgenv: only probe config on root dev (opt-out)
ebg tools: add option to search on all devices

Makefile.am | 2 +
docs/API.md | 3 ++
env/env_api.c | 19 +++++--
env/env_api_fat.c | 4 +-
env/env_config_partitions.c | 90 +++++++++++++++++++++++++++++++--
include/ebgenv.h | 9 ++++
include/ebgpart.h | 2 +-
include/env_api.h | 2 +-
include/env_config_partitions.h | 2 +-
include/loader_interface.h | 25 +++++++++
kernel-stub/main.c | 11 ++++
loader_interface.c | 64 +++++++++++++++++++++++
main.c | 12 +++++
tools/bg_envtools.c | 4 ++
tools/bg_envtools.h | 4 ++
tools/bg_printenv.c | 7 ++-
tools/bg_setenv.c | 8 ++-
tools/ebgpart.c | 27 ++++++----
18 files changed, 271 insertions(+), 24 deletions(-)
create mode 100644 include/loader_interface.h
create mode 100644 loader_interface.c

--
2.39.2

[Felix Moessbauer]

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.mo...@siemens.com>
---
Makefile.am | 2 ++
include/loader_interface.h | 25 +++++++++++++++
kernel-stub/main.c | 11 +++++++
loader_interface.c | 64 ++++++++++++++++++++++++++++++++++++++
main.c | 12 +++++++
5 files changed, 114 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.mo...@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..787c2fb
--- /dev/null
+++ b/loader_interface.c
@@ -0,0 +1,64 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2023
+ *
+ * Authors:
+ * Felix Moessbauer <felix.mo...@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);
+ }
+ INFO(L"LoaderDevicePartUUID=%s\n", boot_medium_uuidstr);
+ FreePool(boot_medium_uuidstr);
FreePool(payload_dev_path);
FreePool(boot_medium_path);

--
2.39.2

[Felix Moessbauer]

This patch limits the probing of ebg config environments to the block
device the bootloader was started on. This setting can be overwritten by
configuring using the new API ebg_set_opt before creating / opening the
environment.

The boot block device is queried using the systemd boot loader
interface, or more precisely the LoaderDevicePartUUID efi variable. In
case this variable is not set, or the value does not point to a valid
device, all devices are searched.

We believe that changing the default to "this device" only is a
reasonable decision, because:

- parsing only the current device makes updates more robust, as no
environments from other disks (e.g. USB drives) are considered.
- more efficient as no hanging / slow devices are queried
- less errors in the syslog

To support the recovery use-case, tools that use libebgenv can
explicitly enable the search on all devices.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

env/env_api.c | 19 +++++--
env/env_api_fat.c | 4 +-
env/env_config_partitions.c | 90 +++++++++++++++++++++++++++++++--
include/ebgenv.h | 9 ++++
include/ebgpart.h | 2 +-
include/env_api.h | 2 +-
include/env_config_partitions.h | 2 +-

tools/bg_printenv.c | 3 +-
tools/bg_setenv.c | 3 +-
tools/ebgpart.c | 27 ++++++----
10 files changed, 137 insertions(+), 24 deletions(-)

diff --git a/env/env_api.c b/env/env_api.c
index 11683bd..5c8877c 100644
--- a/env/env_api.c
+++ b/env/env_api.c
@@ -1,10 +1,11 @@
/*
* EFI Boot Guard
*
- * Copyright (c) Siemens AG, 2017
+ * Copyright (c) Siemens AG, 2017-2023
*
* Authors:
* Andreas Reichel <andreas.r...@siemens.com>

+ * Felix Moessbauer <felix.mo...@siemens.com>

*

* This work is licensed under the terms of the GNU GPL, version 2. See

* the COPYING file in the top-level directory.

@@ -56,6 +57,18 @@ char16_t *str8to16(char16_t *buffer, const char *src)
return tmp;
}

+int ebg_set_opt(ebgenv_t *e, ebg_opt_t opt, void *value)
+{
+ switch (opt) {
+ case EBG_OPT_SEARCH_ALL_DEVICES:
+ e->opts.search_all_devs = (bool)value;
+ break;
+ default:
+ return EINVAL;
+ }
+ return 0;
+}
+
void ebg_beverbose(ebgenv_t __attribute__((unused)) *e, bool v)
{
bgenv_be_verbose(v);
@@ -63,7 +76,7 @@ void ebg_beverbose(ebgenv_t __attribute__((unused)) *e, bool v)

int ebg_env_create_new(ebgenv_t *e)
{
- if (!bgenv_init()) {
+ if (!bgenv_init(e)) {
return EIO;
}

@@ -96,7 +109,7 @@ int ebg_env_create_new(ebgenv_t *e)

int ebg_env_open_current(ebgenv_t *e)
{
- if (!bgenv_init()) {
+ if (!bgenv_init(e)) {
return EIO;
}

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 18206a7..9b8ca72 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -168,13 +168,13 @@ BG_ENVDATA __attribute__((weak)) envdata[ENV_NUM_CONFIG_PARTS];

static bool initialized;

-bool bgenv_init(void)
+bool bgenv_init(ebgenv_t *e)
{
if (initialized) {
return true;
}
/* enumerate all config partitions */
- if (!probe_config_partitions(config_parts)) {
+ if (!probe_config_partitions(config_parts, e->opts.search_all_devs)) {
VERBOSE(stderr, "Error finding config partitions.\n");
return false;
}
diff --git a/env/env_config_partitions.c b/env/env_config_partitions.c
index be52d7f..bd3c7af 100644
--- a/env/env_config_partitions.c
+++ b/env/env_config_partitions.c
@@ -1,10 +1,11 @@
/*
* EFI Boot Guard
*
- * Copyright (c) Siemens AG, 2017
+ * Copyright (c) Siemens AG, 2017-2023
*
* Authors:
* Andreas Reichel <andreas.r...@siemens.com>

+ * Felix Moessbauer <felix.mo...@siemens.com>

*

* This work is licensed under the terms of the GNU GPL, version 2. See

* the COPYING file in the top-level directory.

@@ -17,17 +18,100 @@
#include "env_config_partitions.h"
#include "env_config_file.h"

-bool probe_config_partitions(CONFIG_PART *cfgpart)
+#define LOADER_PROT_VENDOR_GUID "4a67b082-0a4c-41cf-b6c7-440b29bb8c4f"
+#define GUID_LEN_CHARS 36
+#define EFI_ATTR_LEN_IN_WCHAR 2
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+
+/**
+ * Read the ESP UUID from the efivars. This only works if the bootloader
+ * implements the LoaderDevicePartUUID from the systemd bootloader interface
+ * spec. Returns a device name (e.g. sda) or NULL. The returned string needs
+ * to be freed by the caller.
+ */
+static char *get_rootdev_from_efi(void)
+{
+ const char *vendor_guid = LOADER_PROT_VENDOR_GUID;
+ const char *basepath = "/sys/firmware/efi/efivars/";
+ char part_uuid[GUID_LEN_CHARS + 1];
+ FILE *f = 0;
+ union {
+ char aschar[512];
+ char16_t aswchar[256];
+ } buffer;
+
+ // read LoaderDevicePartUUID efi variable
+ snprintf(buffer.aschar, sizeof(buffer.aschar),
+ "%s/LoaderDevicePartUUID-%s", basepath, vendor_guid);
+ if (!(f = fopen(buffer.aschar, "r"))) {
+ VERBOSE(stderr, "Error, cannot access efi var at %s.\n",
+ buffer.aschar);
+ return NULL;
+ }
+ const size_t readnb = fread(buffer.aswchar, sizeof(*buffer.aswchar),
+ ARRAY_SIZE(buffer.aswchar), f);
+ if (readnb != GUID_LEN_CHARS + EFI_ATTR_LEN_IN_WCHAR) {
+ VERBOSE(stderr, "Data in LoaderDevicePartUUID not valid\n");
+ fclose(f);
+ return NULL;
+ }
+ fclose(f);
+
+ // convert char16_t to char and lowercase uuid, skip attributes
+ for (int i = 0; i < GUID_LEN_CHARS; i++) {
+ part_uuid[i] = tolower(
+ (char)buffer.aswchar[i + EFI_ATTR_LEN_IN_WCHAR]);
+ }
+ part_uuid[GUID_LEN_CHARS] = '\0';
+
+ // resolve device based on partition uuid
+ snprintf(buffer.aschar, sizeof(buffer.aschar),
+ "/dev/disk/by-partuuid/%s", part_uuid);
+ char *devpath = realpath(buffer.aschar, NULL);
+ if (!devpath) {
+ VERBOSE(stderr, "Error, no disk in %s\n", buffer.aschar);
+ return NULL;
+ }
+ VERBOSE(stdout, "resolved ESP to %s\n", devpath);
+ // get disk name from path
+ char *partition = strrchr(devpath, '/') + 1;
+
+ // resolve parent device. As the ESP must be a primary partition, the
+ // parent is the block device.
+ snprintf(buffer.aschar, sizeof(buffer.aschar), "/sys/class/block/%s/..",
+ partition);
+ free(devpath);
+
+ // resolve to e.g. /sys/devices/pci0000:00/0000:00:1f.2/<...>/block/sda
+ char *blockpath = realpath(buffer.aschar, NULL);
+ char *_blockdev = strrchr(blockpath, '/') + 1;
+ char *blockdev = strdup(_blockdev);
+ free(blockpath);
+ return blockdev;
+}
+
+bool probe_config_partitions(CONFIG_PART *cfgpart, bool search_all_devs)
{
PedDevice *dev = NULL;
char devpath[4096];
+ char *rootdev = NULL;
int count = 0;

if (!cfgpart) {
return false;
}

- ped_device_probe_all();
+ if (!search_all_devs) {
+ if (!(rootdev = get_rootdev_from_efi())) {
+ VERBOSE(stderr, "Warning, could not determine root "
+ "dev. Search on all devices\n");
+ } else {
+ VERBOSE(stdout, "Limit probing to disk %s\n", rootdev);
+ }
+ }
+
+ ped_device_probe_all(rootdev);
+ free(rootdev);

while ((dev = ped_device_get_next(dev))) {
printf_debug("Device: %s\n", dev->model);
diff --git a/include/ebgenv.h b/include/ebgenv.h
index a60c322..a78eb0c 100644
--- a/include/ebgenv.h
+++ b/include/ebgenv.h
@@ -36,11 +36,20 @@

#define USERVAR_STANDARD_TYPE_MASK ((1ULL << 32) - 1)

+typedef struct {
+ bool search_all_devs;
+} ebgenv_opts_t;
+
typedef struct {
void *bgenv;
void *gc_registry;
+ ebgenv_opts_t opts;
} ebgenv_t;

+typedef enum { EBG_OPT_SEARCH_ALL_DEVICES } ebg_opt_t;
+
+int ebg_set_opt(ebgenv_t *e, ebg_opt_t opt, void *value);
+
/** @brief Tell the library to output information for the user.
* @param e A pointer to an ebgenv_t context.
* @param v A boolean to set verbosity.
diff --git a/include/ebgpart.h b/include/ebgpart.h
index 6687156..65b2d1a 100644
--- a/include/ebgpart.h
+++ b/include/ebgpart.h
@@ -128,7 +128,7 @@ typedef struct _PedDisk {
PedPartition *part_list;
} PedDisk;

-void ped_device_probe_all(void);
+void ped_device_probe_all(char *rootdev);
PedDevice *ped_device_get_next(const PedDevice *dev);
PedDisk *ped_disk_new(const PedDevice *dev);
PedPartition *ped_disk_next_partition(const PedDisk *pd,
diff --git a/include/env_api.h b/include/env_api.h
index 7999727..371a762 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -81,7 +81,7 @@ extern char16_t *str8to16(char16_t *buffer, const char *src);

extern uint32_t bgenv_crc32(uint32_t, const void *, size_t);

-extern bool bgenv_init(void);
+extern bool bgenv_init(ebgenv_t *e);
extern void bgenv_finalize(void);
extern BGENV *bgenv_open_by_index(uint32_t index);
extern BGENV *bgenv_open_oldest(void);
diff --git a/include/env_config_partitions.h b/include/env_config_partitions.h
index e676d93..e2a0466 100644
--- a/include/env_config_partitions.h
+++ b/include/env_config_partitions.h
@@ -17,4 +17,4 @@
#include <stdbool.h>
#include "env_api.h"

-bool probe_config_partitions(CONFIG_PART *cfgpart);
+bool probe_config_partitions(CONFIG_PART *cfgpart, bool search_all_devs);
diff --git a/tools/bg_printenv.c b/tools/bg_printenv.c
index 9c52505..abf745b 100644
--- a/tools/bg_printenv.c
+++ b/tools/bg_printenv.c
@@ -344,7 +344,8 @@ error_t bg_printenv(int argc, char **argv)
}

/* not in file mode */
- if (!bgenv_init()) {
+ ebgenv_t ebgenv;
+ if (!bgenv_init(&ebgenv)) {
fprintf(stderr, "Error initializing FAT environment.\n");
return 1;
}
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
index 375cad8..27706ac 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -418,7 +418,8 @@ error_t bg_setenv(int argc, char **argv)
}

/* not in file mode */
- if (!bgenv_init()) {
+ ebgenv_t ebgenv;
+ if (!bgenv_init(&ebgenv)) {
fprintf(stderr, "Error initializing FAT environment.\n");
return 1;
}
diff --git a/tools/ebgpart.c b/tools/ebgpart.c
index 6f60c24..fc754df 100644
--- a/tools/ebgpart.c
+++ b/tools/ebgpart.c
@@ -416,9 +416,9 @@ static int get_major_minor(char *filename, unsigned int *major, unsigned int *mi
return 0;
}

-void ped_device_probe_all(void)
+void ped_device_probe_all(char *rootdev)
{
- struct dirent *sysblockfile;
+ struct dirent *sysblockfile = NULL;
char fullname[DEV_FILENAME_LEN+16];

DIR *sysblockdir = opendir(SYSBLOCKDIR);
@@ -429,16 +429,21 @@ void ped_device_probe_all(void)

/* get all files from sysblockdir */
do {
- sysblockfile = readdir(sysblockdir);
- if (!sysblockfile) {
- break;
- }
- if (strcmp(sysblockfile->d_name, ".") == 0 ||
- strcmp(sysblockfile->d_name, "..") == 0) {
- continue;
+ char *devname = rootdev;
+ if (!rootdev) {
+ sysblockfile = readdir(sysblockdir);
+ if (!sysblockfile) {
+ break;
+ }
+ if (strcmp(sysblockfile->d_name, ".") == 0 ||
+ strcmp(sysblockfile->d_name, "..") == 0) {
+ continue;
+ }
+ devname = sysblockfile->d_name;
}
+
(void)snprintf(fullname, sizeof(fullname), "/sys/block/%s/dev",
- sysblockfile->d_name);
+ devname);
/* Get major and minor revision from /sys/block/sdX/dev */
unsigned int fmajor, fminor;
if (get_major_minor(fullname, &fmajor, &fminor) < 0) {
@@ -449,7 +454,7 @@ void ped_device_probe_all(void)
fmajor, fminor, fullname);
/* Check if this file is really in the dev directory */
(void)snprintf(fullname, sizeof(fullname), "%s/%s", DEVDIR,
- sysblockfile->d_name);
+ devname);
struct stat statbuf;
if (stat(fullname, &statbuf) == -1) {
/* Node with same name not found in /dev, thus search
--
2.39.2

[Felix Moessbauer]

With the recent change in libebgenv to only search on the current boot
device (default), the provided tools could no longer be used for
recovery. To solve this, we add a command line option to all tools to
explicitly enable the system-wide search for ebg environments.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

docs/API.md | 3 +++
tools/bg_envtools.c | 4 ++++
tools/bg_envtools.h | 4 ++++
tools/bg_printenv.c | 4 ++++
tools/bg_setenv.c | 5 +++++
5 files changed, 20 insertions(+)

diff --git a/docs/API.md b/docs/API.md
index 42e2963..cde4cef 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -50,6 +50,7 @@ and sets it to the testing state:
int main(void)
{
ebgenv_t e;
+ memset(&e, 0, sizeof(ebgenv_t));

ebg_env_create_new(&e);
ebg_env_set(&e, "kernelfile", "vmlinux-new");
@@ -76,6 +77,7 @@ modifies the kernel file name:
int main(void)
{
ebgenv_t e;
+ memset(&e, 0, sizeof(ebgenv_t));

ebg_env_open_current(&e);
ebg_env_set(&e, "kernelfile", "vmlinux-new");
@@ -93,6 +95,7 @@ int main(void)
int main(void)
{
ebgenv_t e;
+ memset(&e, 0, sizeof(ebgenv_t));

ebg_env_open_current(&e);

diff --git a/tools/bg_envtools.c b/tools/bg_envtools.c
index 2d29e46..5c8128f 100644
--- a/tools/bg_envtools.c
+++ b/tools/bg_envtools.c
@@ -67,6 +67,10 @@ error_t parse_common_opt(int key, char *arg, bool compat_mode,
bool found = false;
int i;
switch (key) {
+ case 'A':
+ found = true;
+ arguments->search_all_devices = true;
+ break;
case 'f':
found = true;
free(arguments->envfilepath);
diff --git a/tools/bg_envtools.h b/tools/bg_envtools.h
index 8081d86..376041c 100644
--- a/tools/bg_envtools.h
+++ b/tools/bg_envtools.h
@@ -35,6 +35,8 @@
"Set environment partition to use. If no partition is " \
"specified, the one with the smallest revision value above " \
"zero is selected.") \
+ , OPT("all", 'A', 0, 0, \
+ "search on all devices instead of root device only") \
, OPT("verbose", 'v', 0, 0, "Be verbose") \
, OPT("version", 'V', 0, 0, "Print version")

@@ -46,6 +48,8 @@ struct arguments_common {
* was specified. */
int which_part;
bool part_specified;
+ /* inspect all devices for bootenvs instead of current root only */
+ bool search_all_devices;
};

int parse_int(char *arg);
diff --git a/tools/bg_printenv.c b/tools/bg_printenv.c
index abf745b..db37826 100644
--- a/tools/bg_printenv.c
+++ b/tools/bg_printenv.c
@@ -345,6 +345,10 @@ error_t bg_printenv(int argc, char **argv)

/* not in file mode */

ebgenv_t ebgenv;
+ memset(&ebgenv, 0, sizeof(ebgenv_t));
+ if (arguments.common.search_all_devices) {
+ ebg_set_opt(&ebgenv, EBG_OPT_SEARCH_ALL_DEVICES, (void *)1);

+ }
if (!bgenv_init(&ebgenv)) {
fprintf(stderr, "Error initializing FAT environment.\n");
return 1;
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c

index 27706ac..0024e9c 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -115,6 +115,7 @@ newaction_nomem:
static void journal_process_action(BGENV *env, struct env_action *action)
{
ebgenv_t e;
+ memset(&e, 0, sizeof(ebgenv_t));

switch (action->task) {
case ENV_TASK_SET:
@@ -419,6 +420,10 @@ error_t bg_setenv(int argc, char **argv)

/* not in file mode */

ebgenv_t ebgenv;
+ memset(&ebgenv, 0, sizeof(ebgenv_t));
+ if (arguments.common.search_all_devices) {
+ ebg_set_opt(&ebgenv, EBG_OPT_SEARCH_ALL_DEVICES, (void *)1);

+ }
if (!bgenv_init(&ebgenv)) {
fprintf(stderr, "Error initializing FAT environment.\n");
return 1;

--
2.39.2

[Jan Kiszka]

This is a non-persistent variable, right?

> + }
> + 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;

Please format properly: return belongs into its own line.

> +
> + 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;

Same here.

Jan

--
Siemens AG, Technology
Linux Expert Center

[Jan Kiszka]

On 16.10.23 04:48, Felix Moessbauer wrote:

An abstraction of a single instance - always hard to get right. I'm not
sure if we will see more of these, so let's not try to be smart and
avoid this abstraction for now. It's imbalanced anyway (not getter, only
a setter).

> +
> void ebg_beverbose(ebgenv_t __attribute__((unused)) *e, bool v)
> {
> bgenv_be_verbose(v);
> @@ -63,7 +76,7 @@ void ebg_beverbose(ebgenv_t __attribute__((unused)) *e, bool v)
>
> int ebg_env_create_new(ebgenv_t *e)
> {
> - if (!bgenv_init()) {
> + if (!bgenv_init(e)) {
> return EIO;
> }
>
> @@ -96,7 +109,7 @@ int ebg_env_create_new(ebgenv_t *e)
>
> int ebg_env_open_current(ebgenv_t *e)
> {
> - if (!bgenv_init()) {
> + if (!bgenv_init(e)) {
> return EIO;
> }
>
> diff --git a/env/env_api_fat.c b/env/env_api_fat.c
> index 18206a7..9b8ca72 100644
> --- a/env/env_api_fat.c
> +++ b/env/env_api_fat.c
> @@ -168,13 +168,13 @@ BG_ENVDATA __attribute__((weak)) envdata[ENV_NUM_CONFIG_PARTS];
>
> static bool initialized;
>
> -bool bgenv_init(void)
> +bool bgenv_init(ebgenv_t *e)

Add the bool as argument here.

Close before the if, avoids repeating that statement.

Just move this bool the ebgenv_t.

[Jan Kiszka]

On 16.10.23 04:48, Felix Moessbauer wrote:

This is really a weird API: Calling ebg_set_opt on an env that has not
been initialized. Another reason to pass search-all-devices as option ov
bgenv_init.

Jan

> fprintf(stderr, "Error initializing FAT environment.\n");
> return 1;
> diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
> index 27706ac..0024e9c 100644
> --- a/tools/bg_setenv.c
> +++ b/tools/bg_setenv.c
> @@ -115,6 +115,7 @@ newaction_nomem:
> static void journal_process_action(BGENV *env, struct env_action *action)
> {
> ebgenv_t e;
> + memset(&e, 0, sizeof(ebgenv_t));
>
> switch (action->task) {
> case ENV_TASK_SET:
> @@ -419,6 +420,10 @@ error_t bg_setenv(int argc, char **argv)
>
> /* not in file mode */
> ebgenv_t ebgenv;
> + memset(&ebgenv, 0, sizeof(ebgenv_t));
> + if (arguments.common.search_all_devices) {
> + ebg_set_opt(&ebgenv, EBG_OPT_SEARCH_ALL_DEVICES, (void *)1);
> + }
> if (!bgenv_init(&ebgenv)) {
> fprintf(stderr, "Error initializing FAT environment.\n");
> return 1;

--

[Jan Kiszka]

On 16.10.23 04:48, Felix Moessbauer wrote:

Thanks for this improvement. Is there a chance for a test case as well?

Jan

[Storm, Christian]

Hi,

> 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).

Just a note:
We deliberately decided *not* to base the state-keeping stuff on EFI variables
back then due to their questionable robustness ― at least back then when
EFI Boot Guard was initiated. If that is still true, we should be prepared
to do proper decisions without this or find an other more robust place to
store this information. If it's no longer true, then we may think about
using EFI variables for more than this, e.g., also for other information
currently stored in an ebgenv...

> 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.

Kind regards,
Christian

--
Dr. Christian Storm
Siemens AG, Technology, T CED SES-DE
Otto-Hahn-Ring 6, 81739 München, Germany

[Jan Kiszka]

On 16.10.23 08:55, Storm, Christian (T CED SES-DE) wrote:
> Hi,
>
>> 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).
>
> Just a note:
> We deliberately decided *not* to base the state-keeping stuff on EFI variables
> back then due to their questionable robustness ― at least back then when
> EFI Boot Guard was initiated. If that is still true, we should be prepared
> to do proper decisions without this or find an other more robust place to
> store this information. If it's no longer true, then we may think about
> using EFI variables for more than this, e.g., also for other information
> currently stored in an ebgenv...

My understanding is that this var is runtime-only, not persistent. But
this is waiting for confirmation.

Jan

>
>
>> 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.
>
>
> Kind regards,
> Christian
>

--

[MOESSBAUER, Felix]

Yes, exactly. The variable is set with EFI_VARIABLE_BOOTSERVICE_ACCESS
| EFI_VARIABLE_RUNTIME_ACCESS, so it is runtime-only. Apart from the
wear-out concerns, this is also important to not spill the value into
another boot with a different loader that does not support this.

Felix

[MOESSBAUER, Felix]

The reason why I choose this is to avoid breaking the API. If we
forward this variable, we also need to change the ebg_env_open_current
and ebg_env_create_new interfaces. We could use a global variable to
track this (similar to the verbose variable), but that means we cannot
control this setting on a per-instance basis.

>
> > +
> >  void ebg_beverbose(ebgenv_t __attribute__((unused)) *e, bool v)
> >  {
> >         bgenv_be_verbose(v);
> > @@ -63,7 +76,7 @@ void ebg_beverbose(ebgenv_t
> > __attribute__((unused)) *e, bool v)
> >  
> >  int ebg_env_create_new(ebgenv_t *e)
> >  {
> > -       if (!bgenv_init()) {
> > +       if (!bgenv_init(e)) {
> >                 return EIO;
> >         }
> >  
> > @@ -96,7 +109,7 @@ int ebg_env_create_new(ebgenv_t *e)
> >  
> >  int ebg_env_open_current(ebgenv_t *e)
> >  {
> > -       if (!bgenv_init()) {
> > +       if (!bgenv_init(e)) {
> >                 return EIO;
> >         }
> >  
> > diff --git a/env/env_api_fat.c b/env/env_api_fat.c
> > index 18206a7..9b8ca72 100644
> > --- a/env/env_api_fat.c
> > +++ b/env/env_api_fat.c
> > @@ -168,13 +168,13 @@ BG_ENVDATA __attribute__((weak))
> > envdata[ENV_NUM_CONFIG_PARTS];
> >  
> >  static bool initialized;
> >  
> > -bool bgenv_init(void)
> > +bool bgenv_init(ebgenv_t *e)
>
> Add the bool as argument here.

This is not public, so yes, here I can change it.
Still, the main issue remains regarding how to inject this value into
the libebgenv without breaking the API.

Felix

>
> >  {

[Felix Moessbauer]

Dear Devs,

this series changes how configuration environments are located.
By that, accidential collisions with environments from other media
(e.g. USB drives) can be avoided. For the recovery use-case, we
provide an option to override this. In addition, this is a first step
towards integrating with the systemd boot loader interface.

The core idea is to forward the boot device information from the
bootloader to the userspace libraries by using EFI variables. This
is required, as properly locating the block device of the config
partitions is tricky, just based on the device the rootfs is on.
Consider the following situations:

- rootfs on different disk
- rootfs on device mapper or mdadm

As we want to stick to the systemd boot loader interface, we do
not forward the UUIDs of the config envs, but just of the partition
the loader was started from (this information is provided by EFI).
Usually this is the UUID of the ESP. In userspace, we then resolve the
backing / parent device of the ESP. This is trivial, as both partitions
need to reside on the same device. Once we have that information, we
can limit the probing to that device.

Changes since v1:

- change option infrastructure to use single global instead
of modifying the bgenv out parameter.
- port verbose option over to new option infrastructure
- adapt testcases to new interface and add test cases
- integrate updates of tools into library patches
- style fixes

Best regards,
Felix Moessbauer
Siemens AG

Felix Moessbauer (3):
efi: implement systemd boot loader interface
libebgenv: only probe config on root dev (opt-out)

port verbose option over to ebg_set_opt

Makefile.am | 2 +
docs/API.md | 6 ++
env/env_api.c | 42 +++++++++-
env/env_api_fat.c | 6 +-
env/env_config_partitions.c | 90 +++++++++++++++++++++-
include/ebgenv.h | 25 ++++++
include/ebgpart.h | 2 +-
include/env_api.h | 7 +-

include/env_config_partitions.h | 2 +-
include/loader_interface.h | 25 ++++++

kernel-stub/main.c | 11 +++
loader_interface.c | 69 +++++++++++++++++
main.c | 12 +++
tools/bg_envtools.c | 4 +
tools/bg_envtools.h | 4 +
tools/bg_printenv.c | 6 ++
tools/bg_setenv.c | 7 ++
tools/ebgpart.c | 27 ++++---
tools/tests/test_bgenv_init_retval.c | 6 +-
tools/tests/test_ebgenv_api.c | 19 +++++
tools/tests/test_probe_config_file.c | 2 +-
tools/tests/test_probe_config_partitions.c | 2 +-
22 files changed, 346 insertions(+), 30 deletions(-)

create mode 100644 include/loader_interface.h
create mode 100644 loader_interface.c

--
2.39.2

[Felix Moessbauer]

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.mo...@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

+ * Felix Moessbauer <felix.mo...@siemens.com>

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.mo...@siemens.com>

+ }
+ 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;
+ }

--
2.39.2

[Felix Moessbauer]

This patch limits the probing of ebg config environments to the block
device the bootloader was started on. This setting can be overwritten by
configuring using the new API ebg_set_opt before creating / opening the
environment.

The boot block device is queried using the systemd boot loader

interface, or more precisely the LoaderDevicePartUUID efi variable. In
case this variable is not set, or the value does not point to a valid
device, all devices are searched.

We believe that changing the default to "this device" only is a
reasonable decision, because:

- parsing only the current device makes updates more robust, as no
environments from other disks (e.g. USB drives) are considered.
- more efficient as no hanging / slow devices are queried
- less errors in the syslog

To support the recovery use-case, tools that use libebgenv can
explicitly enable the search on all devices.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

docs/API.md | 6 ++
env/env_api.c | 30 +++++++-
env/env_api_fat.c | 4 +-
env/env_config_partitions.c | 90 +++++++++++++++++++++-
include/ebgenv.h | 23 ++++++
include/ebgpart.h | 2 +-
include/env_config_partitions.h | 2 +-

tools/bg_envtools.c | 4 +
tools/bg_envtools.h | 4 +

tools/bg_printenv.c | 3 +
tools/bg_setenv.c | 4 +

tools/ebgpart.c | 27 ++++---
tools/tests/test_bgenv_init_retval.c | 6 +-

tools/tests/test_probe_config_file.c | 2 +-
tools/tests/test_probe_config_partitions.c | 2 +-

15 files changed, 186 insertions(+), 23 deletions(-)

diff --git a/docs/API.md b/docs/API.md
index 42e2963..e20f82d 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -50,6 +50,10 @@ and sets it to the testing state:

int main(void)
{
ebgenv_t e;
+ memset(&e, 0, sizeof(ebgenv_t));

+
+ // optionally set ebg options before creating the environment
+ ebg_set_opt_bool(EBG_OPT_PROBE_ALL_DEVICES, true);

ebg_env_create_new(&e);
ebg_env_set(&e, "kernelfile", "vmlinux-new");

@@ -76,6 +80,7 @@ modifies the kernel file name:

int main(void)
{
ebgenv_t e;
+ memset(&e, 0, sizeof(ebgenv_t));

ebg_env_open_current(&e);
ebg_env_set(&e, "kernelfile", "vmlinux-new");

@@ -93,6 +98,7 @@ int main(void)

int main(void)
{
ebgenv_t e;
+ memset(&e, 0, sizeof(ebgenv_t));

ebg_env_open_current(&e);

diff --git a/env/env_api.c b/env/env_api.c
index 11683bd..49cee3b 100644

--- a/env/env_api.c
+++ b/env/env_api.c
@@ -1,10 +1,11 @@
/*
* EFI Boot Guard
*

- * Copyright (c) Siemens AG, 2017

+ * Copyright (c) Siemens AG, 2017-2023
*
* Authors:
* Andreas Reichel <andreas.r...@siemens.com>

+ * Felix Moessbauer <felix.mo...@siemens.com>

*

* This work is licensed under the terms of the GNU GPL, version 2. See

* the COPYING file in the top-level directory.

@@ -16,6 +17,9 @@
#include "ebgenv.h"
#include "uservars.h"

+/* global EBG options */
+ebgenv_opts_t ebgenv_opts;
+
/* UEFI uses 16-bit wide unicode strings.
* However, wchar_t support functions are fixed to 32-bit wide
* characters in glibc. This code is compiled with
@@ -56,6 +60,30 @@ char16_t *str8to16(char16_t *buffer, const char *src)
return tmp;
}

+int ebg_set_opt_bool(ebg_opt_t opt, bool value)
+{
+ switch (opt) {
+ case EBG_OPT_PROBE_ALL_DEVICES:
+ ebgenv_opts.search_all_devices = value;

+ break;
+ default:
+ return EINVAL;
+ }
+ return 0;
+}

+
+int ebg_get_opt_bool(ebg_opt_t opt, bool *value)
+{
+ switch (opt) {
+ case EBG_OPT_PROBE_ALL_DEVICES:
+ *value = ebgenv_opts.search_all_devices;

+ break;
+ default:
+ return EINVAL;
+ }
+ return 0;
+}

+
void ebg_beverbose(ebgenv_t __attribute__((unused)) *e, bool v)
{
bgenv_be_verbose(v);

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 18206a7..2bfdc4f 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -21,6 +21,7 @@
#include "ebgpart.h"

bool bgenv_verbosity = false;
+extern ebgenv_opts_t ebgenv_opts;

EBGENVKEY bgenv_str2enum(char *key)
{
@@ -174,7 +175,8 @@ bool bgenv_init(void)

return true;
}
/* enumerate all config partitions */
- if (!probe_config_partitions(config_parts)) {
+ if (!probe_config_partitions(config_parts,

+ ebgenv_opts.search_all_devices)) {

VERBOSE(stderr, "Error finding config partitions.\n");
return false;
}
diff --git a/env/env_config_partitions.c b/env/env_config_partitions.c

index be52d7f..358c365 100644

--- a/env/env_config_partitions.c
+++ b/env/env_config_partitions.c
@@ -1,10 +1,11 @@
/*
* EFI Boot Guard
*

- * Copyright (c) Siemens AG, 2017

+ * Copyright (c) Siemens AG, 2017-2023
*
* Authors:
* Andreas Reichel <andreas.r...@siemens.com>

+ * Felix Moessbauer <felix.mo...@siemens.com>

*

* This work is licensed under the terms of the GNU GPL, version 2. See

* the COPYING file in the top-level directory.

+ return NULL;
+ }
+ const size_t readnb = fread(buffer.aswchar, sizeof(*buffer.aswchar),

+ ARRAY_SIZE(buffer.aswchar), f);
+ if (readnb != GUID_LEN_CHARS + EFI_ATTR_LEN_IN_WCHAR) {
+ VERBOSE(stderr, "Data in LoaderDevicePartUUID not valid\n");
+ fclose(f);

+ return NULL;
+ }
+ fclose(f);

+
+ // convert char16_t to char and lowercase uuid, skip attributes
+ for (int i = 0; i < GUID_LEN_CHARS; i++) {
+ part_uuid[i] = tolower(
+ (char)buffer.aswchar[i + EFI_ATTR_LEN_IN_WCHAR]);
+ }
+ part_uuid[GUID_LEN_CHARS] = '\0';
+
+ // resolve device based on partition uuid
+ snprintf(buffer.aschar, sizeof(buffer.aschar),
+ "/dev/disk/by-partuuid/%s", part_uuid);
+ char *devpath = realpath(buffer.aschar, NULL);
+ if (!devpath) {
+ VERBOSE(stderr, "Error, no disk in %s\n", buffer.aschar);

+ return NULL;
+ }
+ VERBOSE(stdout, "resolved ESP to %s\n", devpath);

+ // get disk name from path
+ char *partition = strrchr(devpath, '/') + 1;
+
+ // resolve parent device. As the ESP must be a primary partition, the
+ // parent is the block device.
+ snprintf(buffer.aschar, sizeof(buffer.aschar), "/sys/class/block/%s/..",
+ partition);
+ free(devpath);
+
+ // resolve to e.g. /sys/devices/pci0000:00/0000:00:1f.2/<...>/block/sda
+ char *blockpath = realpath(buffer.aschar, NULL);
+ char *_blockdev = strrchr(blockpath, '/') + 1;
+ char *blockdev = strdup(_blockdev);
+ free(blockpath);
+ return blockdev;
+}
+

+bool probe_config_partitions(CONFIG_PART *cfgpart, bool search_all_devices)
{

PedDevice *dev = NULL;
char devpath[4096];
+ char *rootdev = NULL;
int count = 0;

if (!cfgpart) {
return false;
}

- ped_device_probe_all();

+ if (!search_all_devices) {

+ if (!(rootdev = get_rootdev_from_efi())) {
+ VERBOSE(stderr, "Warning, could not determine root "
+ "dev. Search on all devices\n");
+ } else {
+ VERBOSE(stdout, "Limit probing to disk %s\n", rootdev);
+ }
+ }
+
+ ped_device_probe_all(rootdev);
+ free(rootdev);

while ((dev = ped_device_get_next(dev))) {
printf_debug("Device: %s\n", dev->model);
diff --git a/include/ebgenv.h b/include/ebgenv.h

index a60c322..c1eacc2 100644
--- a/include/ebgenv.h
+++ b/include/ebgenv.h
@@ -36,11 +36,34 @@

#define USERVAR_STANDARD_TYPE_MASK ((1ULL << 32) - 1)

+typedef struct {

+ bool search_all_devices;

+} ebgenv_opts_t;
+
typedef struct {
void *bgenv;
void *gc_registry;
+ ebgenv_opts_t opts;
} ebgenv_t;

+typedef enum { EBG_OPT_PROBE_ALL_DEVICES } ebg_opt_t;
+
+/**
+ * @brief Set a global EBG option. Call before creating the ebg env.
+ * @param opt option to set
+ * @param value option value
+ * @return 0 on success
+ */
+int ebg_set_opt_bool(ebg_opt_t opt, bool value);
+
+/**
+ * @brief Get a global EBG option.
+ * @param opt option to set
+ * @param value out variable to retrieve option value
+ * @return 0 on success
+ */
+int ebg_get_opt_bool(ebg_opt_t opt, bool *value);

+
/** @brief Tell the library to output information for the user.
* @param e A pointer to an ebgenv_t context.
* @param v A boolean to set verbosity.
diff --git a/include/ebgpart.h b/include/ebgpart.h
index 6687156..65b2d1a 100644
--- a/include/ebgpart.h
+++ b/include/ebgpart.h
@@ -128,7 +128,7 @@ typedef struct _PedDisk {
PedPartition *part_list;
} PedDisk;

-void ped_device_probe_all(void);
+void ped_device_probe_all(char *rootdev);
PedDevice *ped_device_get_next(const PedDevice *dev);
PedDisk *ped_disk_new(const PedDevice *dev);
PedPartition *ped_disk_next_partition(const PedDisk *pd,

diff --git a/include/env_config_partitions.h b/include/env_config_partitions.h
index e676d93..c4eb465 100644

--- a/include/env_config_partitions.h
+++ b/include/env_config_partitions.h
@@ -17,4 +17,4 @@
#include <stdbool.h>
#include "env_api.h"

-bool probe_config_partitions(CONFIG_PART *cfgpart);

+bool probe_config_partitions(CONFIG_PART *cfgpart, bool search_all_devices);

diff --git a/tools/bg_printenv.c b/tools/bg_printenv.c
index 9c52505..4e11b78 100644
--- a/tools/bg_printenv.c
+++ b/tools/bg_printenv.c
@@ -344,6 +344,9 @@ error_t bg_printenv(int argc, char **argv)

}

/* not in file mode */

+ if (arguments.common.search_all_devices) {
+ ebg_set_opt_bool(EBG_OPT_PROBE_ALL_DEVICES, true);
+ }
if (!bgenv_init()) {

fprintf(stderr, "Error initializing FAT environment.\n");
return 1;
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c

index 375cad8..a322582 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c

@@ -115,6 +115,7 @@ newaction_nomem:
static void journal_process_action(BGENV *env, struct env_action *action)
{
ebgenv_t e;
+ memset(&e, 0, sizeof(ebgenv_t));

switch (action->task) {
case ENV_TASK_SET:

@@ -418,6 +419,9 @@ error_t bg_setenv(int argc, char **argv)

}

/* not in file mode */

+ if (arguments.common.search_all_devices) {
+ ebg_set_opt_bool(EBG_OPT_PROBE_ALL_DEVICES, true);
+ }
if (!bgenv_init()) {

diff --git a/tools/tests/test_bgenv_init_retval.c b/tools/tests/test_bgenv_init_retval.c
index 6f1c9cc..7c9badc 100644
--- a/tools/tests/test_bgenv_init_retval.c
+++ b/tools/tests/test_bgenv_init_retval.c
@@ -26,12 +26,12 @@ static char *devpath = "/dev/nobrain";
bool read_env(CONFIG_PART *part, BG_ENVDATA *env);

Suite *env_api_fat_suite(void);
-bool probe_config_partitions_custom_fake(CONFIG_PART *cfgpart);
+bool probe_config_partitions_custom_fake(CONFIG_PART *cfgpart, bool probe_all);
bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env);

Suite *ebg_test_suite(void);

-bool probe_config_partitions_custom_fake(CONFIG_PART *cfgpart)
+bool probe_config_partitions_custom_fake(CONFIG_PART *cfgpart, bool probe_all)
{
for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
cfgpart[i].devpath = devpath;
@@ -48,7 +48,7 @@ bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env)
return true;
}

-FAKE_VALUE_FUNC(bool, probe_config_partitions, CONFIG_PART *);
+FAKE_VALUE_FUNC(bool, probe_config_partitions, CONFIG_PART *, bool);
FAKE_VALUE_FUNC(bool, read_env, CONFIG_PART *, BG_ENVDATA *);

START_TEST(env_api_fat_test_bgenv_init_retval)
diff --git a/tools/tests/test_probe_config_file.c b/tools/tests/test_probe_config_file.c
index fb5055b..b53207e 100644
--- a/tools/tests/test_probe_config_file.c
+++ b/tools/tests/test_probe_config_file.c
@@ -132,7 +132,7 @@ void delete_temp_files(void)
}
}

-FAKE_VOID_FUNC(ped_device_probe_all);
+FAKE_VOID_FUNC(ped_device_probe_all, char *);
FAKE_VALUE_FUNC(PedDevice *, ped_device_get_next, const PedDevice *);
FAKE_VALUE_FUNC(char *, get_mountpoint, char *);

diff --git a/tools/tests/test_probe_config_partitions.c b/tools/tests/test_probe_config_partitions.c
index da60b95..c33eefd 100644
--- a/tools/tests/test_probe_config_partitions.c
+++ b/tools/tests/test_probe_config_partitions.c
@@ -37,7 +37,7 @@ bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env)
}

FAKE_VALUE_FUNC(bool, read_env, CONFIG_PART *, BG_ENVDATA *);
-FAKE_VOID_FUNC(ped_device_probe_all);
+FAKE_VOID_FUNC(ped_device_probe_all, char *);
FAKE_VALUE_FUNC(PedDevice *, ped_device_get_next, const PedDevice *);

START_TEST(env_api_fat_test_probe_config_partitions)
--
2.39.2

[Felix Moessbauer]

This patch deprecated the ebg_beverbose function and ports the logic
over to the ebg_set_opt_bool infrastructure. By that, the interface can
be simplified.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

env/env_api.c | 12 +++++++++++-
env/env_api_fat.c | 2 --
include/ebgenv.h | 4 +++-
include/env_api.h | 7 +++----
tools/bg_printenv.c | 3 +++
tools/bg_setenv.c | 3 +++
tools/tests/test_ebgenv_api.c | 19 +++++++++++++++++++
7 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/env/env_api.c b/env/env_api.c
index 49cee3b..e94cb46 100644
--- a/env/env_api.c
+++ b/env/env_api.c
@@ -66,6 +66,10 @@ int ebg_set_opt_bool(ebg_opt_t opt, bool value)
case EBG_OPT_PROBE_ALL_DEVICES:
ebgenv_opts.search_all_devices = value;
break;
+ case EBG_OPT_VERBOSE:
+ ebgenv_opts.verbose = value;
+ bgenv_be_verbose(value);
+ break;
default:
return EINVAL;
}
@@ -78,6 +82,9 @@ int ebg_get_opt_bool(ebg_opt_t opt, bool *value)
case EBG_OPT_PROBE_ALL_DEVICES:
*value = ebgenv_opts.search_all_devices;
break;
+ case EBG_OPT_VERBOSE:
+ *value = ebgenv_opts.verbose;
+ break;
default:
return EINVAL;
}
@@ -86,7 +93,10 @@ int ebg_get_opt_bool(ebg_opt_t opt, bool *value)

void ebg_beverbose(ebgenv_t __attribute__((unused)) *e, bool v)
{

- bgenv_be_verbose(v);
+ fprintf(stderr,
+ "WARNING: ebg_beverbose is deprecated. "
+ "Use ebg_set_opt_bool(EBG_OPT_VERBOSE, true); instead.");
+ ebg_set_opt_bool(EBG_OPT_VERBOSE, v);
}

int ebg_env_create_new(ebgenv_t *e)
diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 2bfdc4f..3433165 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -20,7 +20,6 @@
#include "test-interface.h"
#include "ebgpart.h"

-bool bgenv_verbosity = false;

extern ebgenv_opts_t ebgenv_opts;

EBGENVKEY bgenv_str2enum(char *key)

@@ -48,7 +47,6 @@ EBGENVKEY bgenv_str2enum(char *key)

void bgenv_be_verbose(bool v)
{
- bgenv_verbosity = v;
ebgpart_beverbose(v);
}

diff --git a/include/ebgenv.h b/include/ebgenv.h
index c1eacc2..b4a7d0b 100644
--- a/include/ebgenv.h
+++ b/include/ebgenv.h
@@ -38,6 +38,7 @@

typedef struct {
bool search_all_devices;
+ bool verbose;
} ebgenv_opts_t;

typedef struct {
@@ -46,7 +47,7 @@ typedef struct {
ebgenv_opts_t opts;
} ebgenv_t;

-typedef enum { EBG_OPT_PROBE_ALL_DEVICES } ebg_opt_t;
+typedef enum { EBG_OPT_PROBE_ALL_DEVICES, EBG_OPT_VERBOSE } ebg_opt_t;

/**

* @brief Set a global EBG option. Call before creating the ebg env.

@@ -67,6 +68,7 @@ int ebg_get_opt_bool(ebg_opt_t opt, bool *value);

/** @brief Tell the library to output information for the user.
* @param e A pointer to an ebgenv_t context.
* @param v A boolean to set verbosity.

+ * @note deprecated. Use \c ebg_set_opt_bool(EBG_OPT_VERBOSE,true) instead
*/
void ebg_beverbose(ebgenv_t *e, bool v);

diff --git a/include/env_api.h b/include/env_api.h
index 7999727..93d29c8 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -42,11 +42,10 @@

#define DEFAULT_TIMEOUT_SEC 30

-extern bool bgenv_verbosity;
+extern ebgenv_opts_t ebgenv_opts;

-#define VERBOSE(o, ...) \
- if (bgenv_verbosity) \
- fprintf(o, __VA_ARGS__)
+#define VERBOSE(o, ...) \
+ if (ebgenv_opts.verbose) fprintf(o, __VA_ARGS__)

typedef enum {
EBGENV_KERNELFILE,
diff --git a/tools/bg_printenv.c b/tools/bg_printenv.c
index 4e11b78..5e5a55f 100644
--- a/tools/bg_printenv.c
+++ b/tools/bg_printenv.c
@@ -347,6 +347,9 @@ error_t bg_printenv(int argc, char **argv)
if (arguments.common.search_all_devices) {
ebg_set_opt_bool(EBG_OPT_PROBE_ALL_DEVICES, true);
}
+ if (arguments.common.verbosity) {
+ ebg_set_opt_bool(EBG_OPT_VERBOSE, true);

+ }
if (!bgenv_init()) {
fprintf(stderr, "Error initializing FAT environment.\n");
return 1;
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c

index a322582..0757e80 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -422,6 +422,9 @@ error_t bg_setenv(int argc, char **argv)
if (arguments.common.search_all_devices) {
ebg_set_opt_bool(EBG_OPT_PROBE_ALL_DEVICES, true);
}
+ if (arguments.common.verbosity) {
+ ebg_set_opt_bool(EBG_OPT_VERBOSE, true);

+ }
if (!bgenv_init()) {
fprintf(stderr, "Error initializing FAT environment.\n");
return 1;

diff --git a/tools/tests/test_ebgenv_api.c b/tools/tests/test_ebgenv_api.c
index cd08e9a..e4f9c46 100644
--- a/tools/tests/test_ebgenv_api.c
+++ b/tools/tests/test_ebgenv_api.c
@@ -83,6 +83,24 @@ int __wrap_bgenv_set(BGENV *env, char *key, uint64_t type, void *buffer,
CONFIG_PART config_parts[ENV_NUM_CONFIG_PARTS];
BG_ENVDATA envdata[ENV_NUM_CONFIG_PARTS];

+START_TEST(ebgenv_api_ebg_env_options)
+{
+ int status;
+ status = ebg_set_opt_bool(EBG_OPT_VERBOSE, true);
+ ck_assert_int_eq(status, 0);
+ bool verbose;
+ status = ebg_get_opt_bool(EBG_OPT_VERBOSE, &verbose);
+ ck_assert_int_eq(status, 0);
+ ck_assert_int_eq(verbose, 1);
+
+ // try invalid option
+ status = ebg_set_opt_bool(0xffff, false);
+ ck_assert_int_ne(status, 0);
+ status = ebg_get_opt_bool(0xffff, &verbose);
+ ck_assert_int_ne(status, 0);
+}
+END_TEST
+
START_TEST(ebgenv_api_ebg_env_create_new)
{
ebgenv_t e;
@@ -665,6 +683,7 @@ Suite *ebg_test_suite(void)

tc_core = tcase_create("Core");

+ tcase_add_test(tc_core, ebgenv_api_ebg_env_options);
tcase_add_test(tc_core, ebgenv_api_ebg_env_create_new);
tcase_add_test(tc_core, ebgenv_api_ebg_env_open_current);
tcase_add_test(tc_core, ebgenv_api_ebg_env_get);
--
2.39.2

[Jan Kiszka]

In theory, this function should not be called much more than once per
application. Still, I would avoid dumping this internal thing to the
console. Let's first of all make the compilation noisy (__attribute__
((deprecated))).

Jan

[Jan Kiszka]

On 16.10.23 12:44, 'Felix Moessbauer' via EFI Boot Guard wrote:

But there is no test case yet that checked if check-all vs.
check-LoaderDevicePartUUID works, is there?

Jan

[Felix Moessbauer]

Dear Devs,

this series changes how configuration environments are located.
By that, accidential collisions with environments from other media
(e.g. USB drives) can be avoided. For the recovery use-case, we
provide an option to override this. In addition, this is a first step
towards integrating with the systemd boot loader interface.

The core idea is to forward the boot device information from the
bootloader to the userspace libraries by using EFI variables. This
is required, as properly locating the block device of the config
partitions is tricky, just based on the device the rootfs is on.
Consider the following situations:

- rootfs on different disk
- rootfs on device mapper or mdadm

As we want to stick to the systemd boot loader interface, we do
not forward the UUIDs of the config envs, but just of the partition
the loader was started from (this information is provided by EFI).
Usually this is the UUID of the ESP. In userspace, we then resolve the
backing / parent device of the ESP. This is trivial, as both partitions
need to reside on the same device. Once we have that information, we
can limit the probing to that device.

Changes since v2:

- add tests for all devs vs. current dev logic
- set deprecated attrib on ebg_beverbose instead of log message

Changes since v1:

- change option infrastructure to use single global instead
of modifying the bgenv out parameter.
- port verbose option over to new option infrastructure
- adapt testcases to new interface and add test cases
- integrate updates of tools into library patches
- style fixes

Best regards,
Felix Moessbauer
Siemens AG

Felix Moessbauer (3):
efi: implement systemd boot loader interface
libebgenv: only probe config on root dev (opt-out)
port verbose option over to ebg_set_opt

Makefile.am | 2 +
docs/API.md | 6 ++
env/env_api.c | 42 +++++++++-

env/env_api_fat.c | 6 +-
env/env_config_partitions.c | 90 +++++++++++++++++++++-
include/ebgenv.h | 27 ++++++-

include/ebgpart.h | 2 +-
include/env_api.h | 7 +-
include/env_config_partitions.h | 2 +-
include/loader_interface.h | 25 ++++++
kernel-stub/main.c | 11 +++
loader_interface.c | 69 +++++++++++++++++
main.c | 12 +++

tools/bg_envtools.c | 4 +
tools/bg_envtools.h | 4 +

tools/bg_printenv.c | 6 ++
tools/bg_setenv.c | 7 ++
tools/ebgpart.c | 27 ++++---

tools/tests/test_bgenv_init_retval.c | 32 +++++++-
tools/tests/test_ebgenv_api.c | 19 +++++

tools/tests/test_probe_config_file.c | 2 +-
tools/tests/test_probe_config_partitions.c | 2 +-

22 files changed, 371 insertions(+), 33 deletions(-)

create mode 100644 include/loader_interface.h
create mode 100644 loader_interface.c

--
2.39.2

[Felix Moessbauer]

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.mo...@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

+ * Felix Moessbauer <felix.mo...@siemens.com>

+ * Copyright (c) Siemens AG, 2023
+ *
+ * Authors:

+ * Felix Moessbauer <felix.mo...@siemens.com>

+ return NULL;
+ }
+

[Felix Moessbauer]

This patch limits the probing of ebg config environments to the block
device the bootloader was started on. This setting can be overwritten by
configuring using the new API ebg_set_opt before creating / opening the
environment.

The boot block device is queried using the systemd boot loader
interface, or more precisely the LoaderDevicePartUUID efi variable. In
case this variable is not set, or the value does not point to a valid
device, all devices are searched.

We believe that changing the default to "this device" only is a
reasonable decision, because:

- parsing only the current device makes updates more robust, as no
environments from other disks (e.g. USB drives) are considered.
- more efficient as no hanging / slow devices are queried
- less errors in the syslog

To support the recovery use-case, tools that use libebgenv can
explicitly enable the search on all devices.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

docs/API.md | 6 ++
env/env_api.c | 30 +++++++-

env/env_api_fat.c | 4 +-
env/env_config_partitions.c | 90 +++++++++++++++++++++-

include/ebgenv.h | 23 ++++++
include/ebgpart.h | 2 +-
include/env_config_partitions.h | 2 +-

tools/bg_envtools.c | 4 +
tools/bg_envtools.h | 4 +

tools/bg_printenv.c | 3 +
tools/bg_setenv.c | 4 +

tools/ebgpart.c | 27 ++++---
tools/tests/test_bgenv_init_retval.c | 32 +++++++-

tools/tests/test_probe_config_file.c | 2 +-
tools/tests/test_probe_config_partitions.c | 2 +-

15 files changed, 211 insertions(+), 24 deletions(-)

+ * Felix Moessbauer <felix.mo...@siemens.com>

*

* This work is licensed under the terms of the GNU GPL, version 2. See

* the COPYING file in the top-level directory.

- * Copyright (c) Siemens AG, 2017
+ * Copyright (c) Siemens AG, 2017-2023
*
* Authors:
* Andreas Reichel <andreas.r...@siemens.com>

+ * Felix Moessbauer <felix.mo...@siemens.com>

*

* This work is licensed under the terms of the GNU GPL, version 2. See

* the COPYING file in the top-level directory.

+ return NULL;
+ }

+ const size_t readnb = fread(buffer.aswchar, sizeof(*buffer.aswchar),
+ ARRAY_SIZE(buffer.aswchar), f);
+ if (readnb != GUID_LEN_CHARS + EFI_ATTR_LEN_IN_WCHAR) {
+ VERBOSE(stderr, "Data in LoaderDevicePartUUID not valid\n");
+ fclose(f);

+ return NULL;
+ }

+ fclose(f);
+
+ // convert char16_t to char and lowercase uuid, skip attributes
+ for (int i = 0; i < GUID_LEN_CHARS; i++) {
+ part_uuid[i] = tolower(
+ (char)buffer.aswchar[i + EFI_ATTR_LEN_IN_WCHAR]);
+ }
+ part_uuid[GUID_LEN_CHARS] = '\0';
+
+ // resolve device based on partition uuid
+ snprintf(buffer.aschar, sizeof(buffer.aschar),
+ "/dev/disk/by-partuuid/%s", part_uuid);
+ char *devpath = realpath(buffer.aschar, NULL);
+ if (!devpath) {
+ VERBOSE(stderr, "Error, no disk in %s\n", buffer.aschar);

+ return NULL;
+ }

index 6f1c9cc..c1d6742 100644
--- a/tools/tests/test_bgenv_init_retval.c
+++ b/tools/tests/test_bgenv_init_retval.c
@@ -15,6 +15,7 @@
#include <stdlib.h>
#include <check.h>
#include <fff.h>
+#include <ebgenv.h>
#include <env_api.h>
#include <env_config_file.h>
#include <env_config_partitions.h>
@@ -24,18 +25,24 @@ DEFINE_FFF_GLOBALS;

static char *devpath = "/dev/nobrain";

bool read_env(CONFIG_PART *part, BG_ENVDATA *env);

+char *get_rootdev_from_efi(void);

Suite *env_api_fat_suite(void);
-bool probe_config_partitions_custom_fake(CONFIG_PART *cfgpart);
+bool probe_config_partitions_custom_fake(CONFIG_PART *cfgpart, bool probe_all);
bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env);

Suite *ebg_test_suite(void);

-bool probe_config_partitions_custom_fake(CONFIG_PART *cfgpart)
+bool probe_config_partitions_custom_fake(CONFIG_PART *cfgpart, bool probe_all)
{

+ char *rootdev = NULL;

+ if (!probe_all) {
+ rootdev = get_rootdev_from_efi();
+ }
for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
- cfgpart[i].devpath = devpath;
+ cfgpart[i].devpath = strdup(devpath);
}
+ free(rootdev);
return true;
}

@@ -48,8 +55,9 @@ bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env)

return true;
}

-FAKE_VALUE_FUNC(bool, probe_config_partitions, CONFIG_PART *);
+FAKE_VALUE_FUNC(bool, probe_config_partitions, CONFIG_PART *, bool);
FAKE_VALUE_FUNC(bool, read_env, CONFIG_PART *, BG_ENVDATA *);

+FAKE_VALUE_FUNC(char *, get_rootdev_from_efi);

START_TEST(env_api_fat_test_bgenv_init_retval)
{
@@ -72,14 +80,30 @@ START_TEST(env_api_fat_test_bgenv_init_retval)
/* Test if bgenv_init succeeds if config partitions are found
*/
RESET_FAKE(probe_config_partitions);
+ RESET_FAKE(get_rootdev_from_efi);

probe_config_partitions_fake.custom_fake = probe_config_partitions_custom_fake;
read_env_fake.custom_fake = read_env_custom_fake;
+ get_rootdev_from_efi_fake.return_val = strdup(devpath);
result = bgenv_init();

ck_assert(probe_config_partitions_fake.call_count == 1);
ck_assert(read_env_fake.call_count == ENV_NUM_CONFIG_PARTS);
+ ck_assert(get_rootdev_from_efi_fake.call_count == 1);
+ ck_assert(result == true);
+ bgenv_finalize();
+
+ RESET_FAKE(probe_config_partitions);
+ RESET_FAKE(get_rootdev_from_efi);
+ probe_config_partitions_fake.custom_fake = probe_config_partitions_custom_fake;
+ get_rootdev_from_efi_fake.return_val = strdup(devpath);
+
+ ebg_set_opt_bool(EBG_OPT_PROBE_ALL_DEVICES, true);
+ result = bgenv_init();
+
+ ck_assert(get_rootdev_from_efi_fake.call_count == 0);
ck_assert(result == true);
+ bgenv_finalize();
}
END_TEST

diff --git a/tools/tests/test_probe_config_file.c b/tools/tests/test_probe_config_file.c
index fb5055b..b53207e 100644
--- a/tools/tests/test_probe_config_file.c
+++ b/tools/tests/test_probe_config_file.c
@@ -132,7 +132,7 @@ void delete_temp_files(void)
}
}

-FAKE_VOID_FUNC(ped_device_probe_all);
+FAKE_VOID_FUNC(ped_device_probe_all, char *);
FAKE_VALUE_FUNC(PedDevice *, ped_device_get_next, const PedDevice *);
FAKE_VALUE_FUNC(char *, get_mountpoint, char *);

diff --git a/tools/tests/test_probe_config_partitions.c b/tools/tests/test_probe_config_partitions.c
index da60b95..c33eefd 100644
--- a/tools/tests/test_probe_config_partitions.c
+++ b/tools/tests/test_probe_config_partitions.c
@@ -37,7 +37,7 @@ bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env)
}

FAKE_VALUE_FUNC(bool, read_env, CONFIG_PART *, BG_ENVDATA *);
-FAKE_VOID_FUNC(ped_device_probe_all);
+FAKE_VOID_FUNC(ped_device_probe_all, char *);
FAKE_VALUE_FUNC(PedDevice *, ped_device_get_next, const PedDevice *);

START_TEST(env_api_fat_test_probe_config_partitions)

--
2.39.2

[Felix Moessbauer]

This patch deprecated the ebg_beverbose function and ports the logic
over to the ebg_set_opt_bool infrastructure. By that, the interface can
be simplified.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

env/env_api.c | 12 ++++++++++--
env/env_api_fat.c | 2 --
include/ebgenv.h | 6 ++++--

include/env_api.h | 7 +++----
tools/bg_printenv.c | 3 +++
tools/bg_setenv.c | 3 +++
tools/tests/test_ebgenv_api.c | 19 +++++++++++++++++++

7 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/env/env_api.c b/env/env_api.c
index 49cee3b..3843a41 100644
--- a/env/env_api.c
+++ b/env/env_api.c

@@ -66,6 +66,10 @@ int ebg_set_opt_bool(ebg_opt_t opt, bool value)
case EBG_OPT_PROBE_ALL_DEVICES:
ebgenv_opts.search_all_devices = value;
break;
+ case EBG_OPT_VERBOSE:
+ ebgenv_opts.verbose = value;
+ bgenv_be_verbose(value);
+ break;
default:
return EINVAL;
}

@@ -78,15 +82,19 @@ int ebg_get_opt_bool(ebg_opt_t opt, bool *value)

case EBG_OPT_PROBE_ALL_DEVICES:
*value = ebgenv_opts.search_all_devices;
break;
+ case EBG_OPT_VERBOSE:
+ *value = ebgenv_opts.verbose;
+ break;
default:
return EINVAL;
}

return 0;
}

-void ebg_beverbose(ebgenv_t __attribute__((unused)) *e, bool v)
+void __attribute__((deprecated))
+ebg_beverbose(ebgenv_t __attribute__((unused)) * e, bool v)
{
- bgenv_be_verbose(v);

+ ebg_set_opt_bool(EBG_OPT_VERBOSE, v);
}

int ebg_env_create_new(ebgenv_t *e)

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 2bfdc4f..3433165 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c

@@ -20,7 +20,6 @@
#include "test-interface.h"
#include "ebgpart.h"

-bool bgenv_verbosity = false;

extern ebgenv_opts_t ebgenv_opts;

EBGENVKEY bgenv_str2enum(char *key)

@@ -48,7 +47,6 @@ EBGENVKEY bgenv_str2enum(char *key)

void bgenv_be_verbose(bool v)
{
- bgenv_verbosity = v;
ebgpart_beverbose(v);
}

diff --git a/include/ebgenv.h b/include/ebgenv.h
index c1eacc2..64bcce2 100644
--- a/include/ebgenv.h
+++ b/include/ebgenv.h

@@ -38,6 +38,7 @@

typedef struct {
bool search_all_devices;
+ bool verbose;
} ebgenv_opts_t;

typedef struct {
@@ -46,7 +47,7 @@ typedef struct {
ebgenv_opts_t opts;
} ebgenv_t;

-typedef enum { EBG_OPT_PROBE_ALL_DEVICES } ebg_opt_t;
+typedef enum { EBG_OPT_PROBE_ALL_DEVICES, EBG_OPT_VERBOSE } ebg_opt_t;

/**

* @brief Set a global EBG option. Call before creating the ebg env.

@@ -67,8 +68,9 @@ int ebg_get_opt_bool(ebg_opt_t opt, bool *value);

/** @brief Tell the library to output information for the user.
* @param e A pointer to an ebgenv_t context.
* @param v A boolean to set verbosity.

+ * @note deprecated. Use \c ebg_set_opt_bool(EBG_OPT_VERBOSE,true) instead
*/

-void ebg_beverbose(ebgenv_t *e, bool v);
+void __attribute__((deprecated)) ebg_beverbose(ebgenv_t *e, bool v);

/** @brief Initialize environment library and open environment. The first
* time this function is called, it will create a new environment with

diff --git a/include/env_api.h b/include/env_api.h
index 7999727..93d29c8 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -42,11 +42,10 @@

#define DEFAULT_TIMEOUT_SEC 30

-extern bool bgenv_verbosity;
+extern ebgenv_opts_t ebgenv_opts;

-#define VERBOSE(o, ...) \
- if (bgenv_verbosity) \
- fprintf(o, __VA_ARGS__)
+#define VERBOSE(o, ...) \
+ if (ebgenv_opts.verbose) fprintf(o, __VA_ARGS__)

typedef enum {
EBGENV_KERNELFILE,

diff --git a/tools/bg_printenv.c b/tools/bg_printenv.c
index 4e11b78..5e5a55f 100644
--- a/tools/bg_printenv.c
+++ b/tools/bg_printenv.c

@@ -347,6 +347,9 @@ error_t bg_printenv(int argc, char **argv)
if (arguments.common.search_all_devices) {
ebg_set_opt_bool(EBG_OPT_PROBE_ALL_DEVICES, true);
}
+ if (arguments.common.verbosity) {

+ ebg_set_opt_bool(EBG_OPT_VERBOSE, true);

+ }
if (!bgenv_init()) {
fprintf(stderr, "Error initializing FAT environment.\n");
return 1;
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c

index a322582..0757e80 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -422,6 +422,9 @@ error_t bg_setenv(int argc, char **argv)

if (arguments.common.search_all_devices) {
ebg_set_opt_bool(EBG_OPT_PROBE_ALL_DEVICES, true);
}
+ if (arguments.common.verbosity) {

+ ebg_set_opt_bool(EBG_OPT_VERBOSE, true);

+ }
if (!bgenv_init()) {
fprintf(stderr, "Error initializing FAT environment.\n");
return 1;

2.39.2

[Jan Kiszka]

Do we still need this? ebg_set_opt_bool does not operate against 'e'.

Jan

[Jan Kiszka]

On 17.10.23 09:01, 'Felix Moessbauer' via EFI Boot Guard wrote:

Nit: We do not need the attribute here. It's sufficient in the declaration.

Jan

[Felix Moessbauer]

Dear Devs,

this series changes how configuration environments are located.
By that, accidential collisions with environments from other media
(e.g. USB drives) can be avoided. For the recovery use-case, we
provide an option to override this. In addition, this is a first step
towards integrating with the systemd boot loader interface.

The core idea is to forward the boot device information from the
bootloader to the userspace libraries by using EFI variables. This
is required, as properly locating the block device of the config
partitions is tricky, just based on the device the rootfs is on.
Consider the following situations:

- rootfs on different disk
- rootfs on device mapper or mdadm

As we want to stick to the systemd boot loader interface, we do
not forward the UUIDs of the config envs, but just of the partition
the loader was started from (this information is provided by EFI).
Usually this is the UUID of the ESP. In userspace, we then resolve the
backing / parent device of the ESP. This is trivial, as both partitions
need to reside on the same device. Once we have that information, we
can limit the probing to that device.

Changes since v3:

- dropped explicit memset(ebgenv) from API docs (as not required)
- only add deprecation attrib to declaration (but not to definition)

Changes since v2:

- add tests for all devs vs. current dev logic
- set deprecated attrib on ebg_beverbose instead of log message

Changes since v1:

- change option infrastructure to use single global instead
of modifying the bgenv out parameter.
- port verbose option over to new option infrastructure
- adapt testcases to new interface and add test cases
- integrate updates of tools into library patches
- style fixes

Best regards,
Felix Moessbauer
Siemens AG

Felix Moessbauer (3):
efi: implement systemd boot loader interface
libebgenv: only probe config on root dev (opt-out)
port verbose option over to ebg_set_opt

Makefile.am | 2 +

docs/API.md | 3 +
env/env_api.c | 41 +++++++++-
env/env_api_fat.c | 6 +-
env/env_config_partitions.c | 90 +++++++++++++++++++++-

include/ebgenv.h | 27 ++++++-
include/ebgpart.h | 2 +-
include/env_api.h | 7 +-
include/env_config_partitions.h | 2 +-
include/loader_interface.h | 25 ++++++
kernel-stub/main.c | 11 +++
loader_interface.c | 69 +++++++++++++++++
main.c | 12 +++

tools/bg_envtools.c | 4 +
tools/bg_envtools.h | 4 +

tools/bg_printenv.c | 6 ++
tools/bg_setenv.c | 7 ++

tools/ebgpart.c | 27 ++++---
tools/tests/test_bgenv_init_retval.c | 32 +++++++-

tools/tests/test_ebgenv_api.c | 19 +++++

tools/tests/test_probe_config_file.c | 2 +-
tools/tests/test_probe_config_partitions.c | 2 +-

22 files changed, 367 insertions(+), 33 deletions(-)

create mode 100644 include/loader_interface.h
create mode 100644 loader_interface.c

--
2.39.2

[Felix Moessbauer]

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.mo...@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

+ * Felix Moessbauer <felix.mo...@siemens.com>

+ * Copyright (c) Siemens AG, 2023
+ *
+ * Authors:

+ * Felix Moessbauer <felix.mo...@siemens.com>

+ return NULL;
+ }
+

[Felix Moessbauer]

This patch limits the probing of ebg config environments to the block
device the bootloader was started on. This setting can be overwritten by
configuring using the new API ebg_set_opt before creating / opening the
environment.

The boot block device is queried using the systemd boot loader
interface, or more precisely the LoaderDevicePartUUID efi variable. In
case this variable is not set, or the value does not point to a valid
device, all devices are searched.

We believe that changing the default to "this device" only is a
reasonable decision, because:

- parsing only the current device makes updates more robust, as no
environments from other disks (e.g. USB drives) are considered.
- more efficient as no hanging / slow devices are queried
- less errors in the syslog

To support the recovery use-case, tools that use libebgenv can
explicitly enable the search on all devices.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

docs/API.md | 3 +
env/env_api.c | 30 +++++++-
env/env_api_fat.c | 4 +-
env/env_config_partitions.c | 90 +++++++++++++++++++++-

include/ebgenv.h | 23 ++++++
include/ebgpart.h | 2 +-
include/env_config_partitions.h | 2 +-

tools/bg_envtools.c | 4 +
tools/bg_envtools.h | 4 +

tools/bg_printenv.c | 3 +
tools/bg_setenv.c | 4 +

tools/ebgpart.c | 27 ++++---
tools/tests/test_bgenv_init_retval.c | 32 +++++++-

tools/tests/test_probe_config_file.c | 2 +-
tools/tests/test_probe_config_partitions.c | 2 +-

15 files changed, 208 insertions(+), 24 deletions(-)

diff --git a/docs/API.md b/docs/API.md
index 42e2963..4b5eb12 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -51,6 +51,9 @@ int main(void)
{
ebgenv_t e;

+ // optionally set ebg options before creating the environment
+ ebg_set_opt_bool(EBG_OPT_PROBE_ALL_DEVICES, true);

+

ebg_env_create_new(&e);
ebg_env_set(&e, "kernelfile", "vmlinux-new");

ebg_env_set(&e, "kernelparams", "root=/dev/bootdevice");
diff --git a/env/env_api.c b/env/env_api.c
index 11683bd..49cee3b 100644
--- a/env/env_api.c
+++ b/env/env_api.c

@@ -1,10 +1,11 @@
/*
* EFI Boot Guard
*
- * Copyright (c) Siemens AG, 2017
+ * Copyright (c) Siemens AG, 2017-2023
*
* Authors:
* Andreas Reichel <andreas.r...@siemens.com>

+ * Felix Moessbauer <felix.mo...@siemens.com>

*

* This work is licensed under the terms of the GNU GPL, version 2. See

* the COPYING file in the top-level directory.

void ebg_beverbose(ebgenv_t __attribute__((unused)) *e, bool v)

{
bgenv_be_verbose(v);
diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 18206a7..2bfdc4f 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c

@@ -21,6 +21,7 @@
#include "ebgpart.h"

bool bgenv_verbosity = false;

+extern ebgenv_opts_t ebgenv_opts;

EBGENVKEY bgenv_str2enum(char *key)
{

@@ -174,7 +175,8 @@ bool bgenv_init(void)
return true;
}
/* enumerate all config partitions */
- if (!probe_config_partitions(config_parts)) {
+ if (!probe_config_partitions(config_parts,
+ ebgenv_opts.search_all_devices)) {
VERBOSE(stderr, "Error finding config partitions.\n");
return false;
}
diff --git a/env/env_config_partitions.c b/env/env_config_partitions.c
index be52d7f..358c365 100644
--- a/env/env_config_partitions.c
+++ b/env/env_config_partitions.c
@@ -1,10 +1,11 @@
/*
* EFI Boot Guard
*

- * Copyright (c) Siemens AG, 2017
+ * Copyright (c) Siemens AG, 2017-2023
*
* Authors:
* Andreas Reichel <andreas.r...@siemens.com>

+ * Felix Moessbauer <felix.mo...@siemens.com>

*

* This work is licensed under the terms of the GNU GPL, version 2. See

* the COPYING file in the top-level directory.

+ return NULL;
+ }

+ const size_t readnb = fread(buffer.aswchar, sizeof(*buffer.aswchar),
+ ARRAY_SIZE(buffer.aswchar), f);
+ if (readnb != GUID_LEN_CHARS + EFI_ATTR_LEN_IN_WCHAR) {
+ VERBOSE(stderr, "Data in LoaderDevicePartUUID not valid\n");
+ fclose(f);

+ return NULL;
+ }

+ fclose(f);
+
+ // convert char16_t to char and lowercase uuid, skip attributes
+ for (int i = 0; i < GUID_LEN_CHARS; i++) {
+ part_uuid[i] = tolower(
+ (char)buffer.aswchar[i + EFI_ATTR_LEN_IN_WCHAR]);
+ }
+ part_uuid[GUID_LEN_CHARS] = '\0';
+
+ // resolve device based on partition uuid
+ snprintf(buffer.aschar, sizeof(buffer.aschar),
+ "/dev/disk/by-partuuid/%s", part_uuid);
+ char *devpath = realpath(buffer.aschar, NULL);
+ if (!devpath) {
+ VERBOSE(stderr, "Error, no disk in %s\n", buffer.aschar);

+ return NULL;
+ }

diff --git a/include/ebgenv.h b/include/ebgenv.h
index a60c322..c1eacc2 100644
--- a/include/ebgenv.h
+++ b/include/ebgenv.h

/** @brief Tell the library to output information for the user.
* @param e A pointer to an ebgenv_t context.
* @param v A boolean to set verbosity.

diff --git a/tools/bg_printenv.c b/tools/bg_printenv.c
index 9c52505..4e11b78 100644
--- a/tools/bg_printenv.c
+++ b/tools/bg_printenv.c

@@ -344,6 +344,9 @@ error_t bg_printenv(int argc, char **argv)
}

/* not in file mode */
+ if (arguments.common.search_all_devices) {

+ ebg_set_opt_bool(EBG_OPT_PROBE_ALL_DEVICES, true);

+ }
if (!bgenv_init()) {
fprintf(stderr, "Error initializing FAT environment.\n");
return 1;
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c

index 375cad8..a322582 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c

@@ -115,6 +115,7 @@ newaction_nomem:
static void journal_process_action(BGENV *env, struct env_action *action)
{
ebgenv_t e;
+ memset(&e, 0, sizeof(ebgenv_t));

switch (action->task) {
case ENV_TASK_SET:

@@ -418,6 +419,9 @@ error_t bg_setenv(int argc, char **argv)

}

/* not in file mode */
+ if (arguments.common.search_all_devices) {

+ ebg_set_opt_bool(EBG_OPT_PROBE_ALL_DEVICES, true);

+ }
if (!bgenv_init()) {
fprintf(stderr, "Error initializing FAT environment.\n");
return 1;

2.39.2

[Felix Moessbauer]

This patch deprecated the ebg_beverbose function and ports the logic
over to the ebg_set_opt_bool infrastructure. By that, the interface can
be simplified.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

env/env_api.c | 11 +++++++++--

env/env_api_fat.c | 2 --
include/ebgenv.h | 6 ++++--
include/env_api.h | 7 +++----
tools/bg_printenv.c | 3 +++
tools/bg_setenv.c | 3 +++
tools/tests/test_ebgenv_api.c | 19 +++++++++++++++++++

7 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/env/env_api.c b/env/env_api.c
index 49cee3b..e9871cc 100644
--- a/env/env_api.c
+++ b/env/env_api.c

@@ -66,6 +66,10 @@ int ebg_set_opt_bool(ebg_opt_t opt, bool value)
case EBG_OPT_PROBE_ALL_DEVICES:
ebgenv_opts.search_all_devices = value;
break;
+ case EBG_OPT_VERBOSE:
+ ebgenv_opts.verbose = value;
+ bgenv_be_verbose(value);
+ break;
default:
return EINVAL;
}

@@ -78,15 +82,18 @@ int ebg_get_opt_bool(ebg_opt_t opt, bool *value)

case EBG_OPT_PROBE_ALL_DEVICES:
*value = ebgenv_opts.search_all_devices;
break;
+ case EBG_OPT_VERBOSE:
+ *value = ebgenv_opts.verbose;
+ break;
default:
return EINVAL;
}
return 0;
}

-void ebg_beverbose(ebgenv_t __attribute__((unused)) *e, bool v)
+void ebg_beverbose(ebgenv_t __attribute__((unused)) * e, bool v)

{
- bgenv_be_verbose(v);
+ ebg_set_opt_bool(EBG_OPT_VERBOSE, v);
}

int ebg_env_create_new(ebgenv_t *e)

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 2bfdc4f..3433165 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c

@@ -20,7 +20,6 @@
#include "test-interface.h"
#include "ebgpart.h"

-bool bgenv_verbosity = false;

extern ebgenv_opts_t ebgenv_opts;

EBGENVKEY bgenv_str2enum(char *key)

@@ -48,7 +47,6 @@ EBGENVKEY bgenv_str2enum(char *key)

void bgenv_be_verbose(bool v)
{
- bgenv_verbosity = v;
ebgpart_beverbose(v);
}

diff --git a/include/ebgenv.h b/include/ebgenv.h
index c1eacc2..64bcce2 100644
--- a/include/ebgenv.h
+++ b/include/ebgenv.h

@@ -38,6 +38,7 @@

typedef struct {
bool search_all_devices;
+ bool verbose;
} ebgenv_opts_t;

typedef struct {
@@ -46,7 +47,7 @@ typedef struct {
ebgenv_opts_t opts;
} ebgenv_t;

-typedef enum { EBG_OPT_PROBE_ALL_DEVICES } ebg_opt_t;
+typedef enum { EBG_OPT_PROBE_ALL_DEVICES, EBG_OPT_VERBOSE } ebg_opt_t;

/**

* @brief Set a global EBG option. Call before creating the ebg env.

@@ -67,8 +68,9 @@ int ebg_get_opt_bool(ebg_opt_t opt, bool *value);

/** @brief Tell the library to output information for the user.
* @param e A pointer to an ebgenv_t context.
* @param v A boolean to set verbosity.

+ * @note deprecated. Use \c ebg_set_opt_bool(EBG_OPT_VERBOSE,true) instead
*/
-void ebg_beverbose(ebgenv_t *e, bool v);
+void __attribute__((deprecated)) ebg_beverbose(ebgenv_t *e, bool v);

/** @brief Initialize environment library and open environment. The first
* time this function is called, it will create a new environment with
diff --git a/include/env_api.h b/include/env_api.h
index 7999727..93d29c8 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -42,11 +42,10 @@

#define DEFAULT_TIMEOUT_SEC 30

-extern bool bgenv_verbosity;
+extern ebgenv_opts_t ebgenv_opts;

-#define VERBOSE(o, ...) \
- if (bgenv_verbosity) \
- fprintf(o, __VA_ARGS__)
+#define VERBOSE(o, ...) \
+ if (ebgenv_opts.verbose) fprintf(o, __VA_ARGS__)

typedef enum {
EBGENV_KERNELFILE,

diff --git a/tools/bg_printenv.c b/tools/bg_printenv.c
index 4e11b78..5e5a55f 100644
--- a/tools/bg_printenv.c
+++ b/tools/bg_printenv.c

@@ -347,6 +347,9 @@ error_t bg_printenv(int argc, char **argv)
if (arguments.common.search_all_devices) {
ebg_set_opt_bool(EBG_OPT_PROBE_ALL_DEVICES, true);
}
+ if (arguments.common.verbosity) {

+ ebg_set_opt_bool(EBG_OPT_VERBOSE, true);

+ }
if (!bgenv_init()) {
fprintf(stderr, "Error initializing FAT environment.\n");
return 1;
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c

index a322582..0757e80 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -422,6 +422,9 @@ error_t bg_setenv(int argc, char **argv)

if (arguments.common.search_all_devices) {
ebg_set_opt_bool(EBG_OPT_PROBE_ALL_DEVICES, true);
}
+ if (arguments.common.verbosity) {

+ ebg_set_opt_bool(EBG_OPT_VERBOSE, true);

+ }
if (!bgenv_init()) {
fprintf(stderr, "Error initializing FAT environment.\n");
return 1;

2.39.2

[Jan Kiszka]

Thanks, applied.

Jan

[Michael Adler]

On Wed, 18 Oct 2023 14:56:16 +0800
Hi Felix,

I think you forgot to add the new CLI options to completion/common.py.
This (and the other *.py files) is used to generate the shell
completions for the userspace tools.

Kind regards,
Michael


--
Michael Adler

Siemens AG
Technology
Connectivity & Edge
Smart Embedded Systems
T CED SES-DE
Otto-Hahn-Ring 6
81739 Munich, Germany

Siemens Aktiengesellschaft: Chairman of the Supervisory Board: Jim
Hagemann Snabe; Managing Board: Roland Busch, Chairman, President and
Chief Executive Officer; Cedrik Neike, Matthias Rebellius, Ralf P.
Thomas, Judith Wiese; Registered offices: Berlin and Munich, Germany;
Commercial registries: Berlin-Charlottenburg, HRB 12300, Munich, HRB
6684; WEEE-Reg.-No. DE 23691322

[Jan Kiszka]

On 18.10.23 12:45, Michael Adler wrote:
> On Wed, 18 Oct 2023 14:56:16 +0800
> Hi Felix,
>
> I think you forgot to add the new CLI options to completion/common.py.
> This (and the other *.py files) is used to generate the shell
> completions for the userspace tools.
>

Good point. Please fix that on top, Felix.

[JEMS EBERHARD HORBEL]

DIRECT SENDER IS HERE LETS DEAL.

JENS EBERHARD



MT103/202 DIRECT WIRE TRANSFER
PAYPAL TRANSFER
CASHAPP TRANSFER
ZELLE TRANSFER
TRANSFER WISE
WESTERN UNION TRANSFER
BITCOIN FLASHING 
BANK ACCOUNT LOADING/FLASHING
IBAN TO IBAN TRANSFER
MONEYGRAM TRANSFER
SLBC PROVIDER
CREDIT CARD TOP UP
SEPA TRANSFER
WIRE TRANSFER
GLOBALPAY INC US

Thanks.


NOTE; ONLY SERIOUS / RELIABLE RECEIVERS CAN CONTACT.

DM ME ON WHATSAPP FOR A SERIOUS DEAL.

+447405129573

[Jan Kiszka]

On 18.10.23 08:56, Felix Moessbauer wrote:

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

[MOESSBAUER, Felix]

I'll have a look. That should be easy to fix.

Felix

>
> Jan
>
> > + INFO(L"LoaderDevicePartUUID=%s\n", boot_medium_uuidstr);
> > + FreePool(boot_medium_uuidstr);
> >   FreePool(payload_dev_path);
> >   FreePool(boot_medium_path);
> >  

--
Siemens AG

Linux Expert Center
Friedrich-Ludwig-Bauer-Str. 3
85748 Garching, Germany