On 17.10.23 09:01, 'Felix Moessbauer' via EFI Boot Guard wrote:
> 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 <[email protected]>
> ---
> 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(-)
>
> 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));
Do we still need this? ebg_set_opt_bool does not operate against 'e'.
Jan
> +
> + // 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 <[email protected]>
> + * Felix Moessbauer <[email protected]>
> *
> * 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 <[email protected]>
> + * Felix Moessbauer <[email protected]>
> *
> * 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_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_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 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;
> 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
> diff --git a/tools/tests/test_bgenv_init_retval.c
> b/tools/tests/test_bgenv_init_retval.c
> 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)
--
Siemens AG, Technology
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 [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/efibootguard-dev/95d4f116-2f30-40e2-8cdf-147746c28b2b%40siemens.com.
