From bb9133bb467f49bdfc236b19a51558634681d2c4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 2 Sep 2022 11:04:51 +0200 Subject: [PATCH 1/8] bootspec: simplify paths + insist they are normalized Inspired by #23913, let's complain if people use paths with ".." in Type #1 bootspec entries. Let's prefix all paths with "/" if it is missing. Let's simplify all paths. let's refuse paths/warn with "..". Fixes: #23913 --- src/shared/bootspec.c | 112 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 99 insertions(+), 13 deletions(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 1a47fff167..90ae7887bf 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -53,6 +53,98 @@ static void boot_entry_free(BootEntry *entry) { strv_free(entry->device_tree_overlay); } +static int mangle_path(const char *field, const char *p, char **ret) { + _cleanup_free_ char *c = NULL; + + assert(field); + assert(p); + assert(ret); + + /* Spec leaves open if prefixed with "/" or not, let's normalize that */ + if (path_is_absolute(p)) + c = strdup(p); + else + c = strjoin("/", p); + if (!c) + return -ENOMEM; + + /* We only reference files, never directories */ + if (endswith(c, "/")) { + log_warning("Path in field '%s' has trailing slash, ignoring: %s", field, c); + *ret = NULL; + return 0; + } + + /* Remove duplicate "/" */ + path_simplify(c); + + /* No ".." or "." or so */ + if (!path_is_normalized(c)) { + log_warning("Path in field '%s' is not normalized, ignoring: %s", field, c); + *ret = NULL; + return 0; + } + + *ret = TAKE_PTR(c); + return 1; +} + +static int parse_path_one(const char *field, char **s, const char *p) { + _cleanup_free_ char *c = NULL; + int r; + + assert(field); + assert(s); + assert(p); + + r = mangle_path(field, p, &c); + if (r <= 0) + return r; + + free_and_replace(*s, c); + return 0; +} + +static int parse_path_strv(const char *field, char ***s, const char *p) { + char *c; + int r; + + assert(field); + assert(s); + assert(p); + + r = mangle_path(field, p, &c); + if (r <= 0) + return r; + + return strv_consume(s, c); +} + +static int parse_path_many(const char *field, char ***s, const char *p) { + _cleanup_strv_free_ char **l = NULL, **f = NULL; + int r; + + l = strv_split(p, NULL); + if (!l) + return -ENOMEM; + + STRV_FOREACH(i, l) { + char *c; + + r = mangle_path(field, *i, &c); + if (r < 0) + return r; + if (r == 0) + continue; + + r = strv_consume(&f, c); + if (r < 0) + return r; + } + + return strv_extend_strv(s, f, /* filter_duplicates= */ false); +} + static int boot_entry_load_type1( FILE *f, const char *root, @@ -147,22 +239,16 @@ static int boot_entry_load_type1( else if (streq(field, "options")) r = strv_extend(&tmp.options, p); else if (streq(field, "linux")) - r = free_and_strdup(&tmp.kernel, p); + r = parse_path_one(field, &tmp.kernel, p); else if (streq(field, "efi")) - r = free_and_strdup(&tmp.efi, p); + r = parse_path_one(field, &tmp.efi, p); else if (streq(field, "initrd")) - r = strv_extend(&tmp.initrd, p); + r = parse_path_strv(field, &tmp.initrd, p); else if (streq(field, "devicetree")) - r = free_and_strdup(&tmp.device_tree, p); - else if (streq(field, "devicetree-overlay")) { - _cleanup_strv_free_ char **l = NULL; - - l = strv_split(p, NULL); - if (!l) - return log_oom(); - - r = strv_extend_strv(&tmp.device_tree_overlay, l, false); - } else { + r = parse_path_one(field, &tmp.device_tree, p); + else if (streq(field, "devicetree-overlay")) + r = parse_path_many(field, &tmp.device_tree_overlay, p); + else { log_notice("%s:%u: Unknown line \"%s\", ignoring.", tmp.path, line, field); continue; } From da8f277c094e72ccfa031bf416f174cc9d7e760e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 2 Sep 2022 11:07:22 +0200 Subject: [PATCH 2/8] bootspec: let's actually use the result of strstrip() for further parsing Also, given we are looking for whitespace as separators, just pass NULL as separators, extract_first_word() defaults to whitespace after all. --- src/shared/bootspec.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 90ae7887bf..c8ebbfbf19 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -205,11 +205,11 @@ static int boot_entry_load_type1( line++; - if (IN_SET(*strstrip(buf), '#', '\0')) + p = strstrip(buf); + if (IN_SET(p[0], '#', '\0')) continue; - p = buf; - r = extract_first_word(&p, &field, " \t", 0); + r = extract_first_word(&p, &field, NULL, 0); if (r < 0) { log_error_errno(r, "Failed to parse config file %s line %u: %m", tmp.path, line); continue; @@ -331,11 +331,11 @@ int boot_loader_read_conf(BootConfig *config, FILE *file, const char *path) { line++; - if (IN_SET(*strstrip(buf), '#', '\0')) + p = strstrip(buf); + if (IN_SET(p[0], '#', '\0')) continue; - p = buf; - r = extract_first_word(&p, &field, " \t", 0); + r = extract_first_word(&p, &field, NULL, 0); if (r < 0) { log_error_errno(r, "Failed to parse config file %s line %u: %m", path, line); continue; From d68c6bea5e45083238f5c6fbe00389e3b1c92999 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 2 Sep 2022 11:04:44 +0200 Subject: [PATCH 3/8] bootspec: port type1 parser to log_syntax() --- src/shared/bootspec.c | 79 ++++++++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 27 deletions(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index c8ebbfbf19..ebb172404b 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -53,7 +53,13 @@ static void boot_entry_free(BootEntry *entry) { strv_free(entry->device_tree_overlay); } -static int mangle_path(const char *field, const char *p, char **ret) { +static int mangle_path( + const char *fname, + unsigned line, + const char *field, + const char *p, + char **ret) { + _cleanup_free_ char *c = NULL; assert(field); @@ -70,7 +76,7 @@ static int mangle_path(const char *field, const char *p, char **ret) { /* We only reference files, never directories */ if (endswith(c, "/")) { - log_warning("Path in field '%s' has trailing slash, ignoring: %s", field, c); + log_syntax(NULL, LOG_WARNING, fname, line, 0, "Path in field '%s' has trailing slash, ignoring: %s", field, c); *ret = NULL; return 0; } @@ -80,7 +86,7 @@ static int mangle_path(const char *field, const char *p, char **ret) { /* No ".." or "." or so */ if (!path_is_normalized(c)) { - log_warning("Path in field '%s' is not normalized, ignoring: %s", field, c); + log_syntax(NULL, LOG_WARNING, fname, line, 0, "Path in field '%s' is not normalized, ignoring: %s", field, c); *ret = NULL; return 0; } @@ -89,7 +95,13 @@ static int mangle_path(const char *field, const char *p, char **ret) { return 1; } -static int parse_path_one(const char *field, char **s, const char *p) { +static int parse_path_one( + const char *fname, + unsigned line, + const char *field, + char **s, + const char *p) { + _cleanup_free_ char *c = NULL; int r; @@ -97,7 +109,7 @@ static int parse_path_one(const char *field, char **s, const char *p) { assert(s); assert(p); - r = mangle_path(field, p, &c); + r = mangle_path(fname, line, field, p, &c); if (r <= 0) return r; @@ -105,7 +117,13 @@ static int parse_path_one(const char *field, char **s, const char *p) { return 0; } -static int parse_path_strv(const char *field, char ***s, const char *p) { +static int parse_path_strv( + const char *fname, + unsigned line, + const char *field, + char ***s, + const char *p) { + char *c; int r; @@ -113,14 +131,20 @@ static int parse_path_strv(const char *field, char ***s, const char *p) { assert(s); assert(p); - r = mangle_path(field, p, &c); + r = mangle_path(fname, line, field, p, &c); if (r <= 0) return r; return strv_consume(s, c); } -static int parse_path_many(const char *field, char ***s, const char *p) { +static int parse_path_many( + const char *fname, + unsigned line, + const char *field, + char ***s, + const char *p) { + _cleanup_strv_free_ char **l = NULL, **f = NULL; int r; @@ -131,7 +155,7 @@ static int parse_path_many(const char *field, char ***s, const char *p) { STRV_FOREACH(i, l) { char *c; - r = mangle_path(field, *i, &c); + r = mangle_path(fname, line, field, *i, &c); if (r < 0) return r; if (r == 0) @@ -199,9 +223,9 @@ static int boot_entry_load_type1( if (r == 0) break; if (r == -ENOBUFS) - return log_error_errno(r, "%s:%u: Line too long", tmp.path, line); + return log_syntax(NULL, LOG_ERR, tmp.path, line, r, "Line too long."); if (r < 0) - return log_error_errno(r, "%s:%u: Error while reading: %m", tmp.path, line); + return log_syntax(NULL, LOG_ERR, tmp.path, line, r, "Error while reading: %m"); line++; @@ -211,18 +235,19 @@ static int boot_entry_load_type1( r = extract_first_word(&p, &field, NULL, 0); if (r < 0) { - log_error_errno(r, "Failed to parse config file %s line %u: %m", tmp.path, line); + log_syntax(NULL, LOG_WARNING, tmp.path, line, r, "Failed to parse, ignoring line: %m"); continue; } if (r == 0) { - log_warning("%s:%u: Bad syntax", tmp.path, line); + log_syntax(NULL, LOG_WARNING, tmp.path, line, 0, "Bad syntax, ignoring line."); continue; } if (isempty(p)) { /* Some fields can reasonably have an empty value. In other cases warn. */ if (!STR_IN_SET(field, "options", "devicetree-overlay")) - log_warning("%s:%u: Field %s without value", tmp.path, line, field); + log_syntax(NULL, LOG_WARNING, tmp.path, line, 0, "Field '%s' without value, ignoring line.", field); + continue; } @@ -239,21 +264,21 @@ static int boot_entry_load_type1( else if (streq(field, "options")) r = strv_extend(&tmp.options, p); else if (streq(field, "linux")) - r = parse_path_one(field, &tmp.kernel, p); + r = parse_path_one(tmp.path, line, field, &tmp.kernel, p); else if (streq(field, "efi")) - r = parse_path_one(field, &tmp.efi, p); + r = parse_path_one(tmp.path, line, field, &tmp.efi, p); else if (streq(field, "initrd")) - r = parse_path_strv(field, &tmp.initrd, p); + r = parse_path_strv(tmp.path, line, field, &tmp.initrd, p); else if (streq(field, "devicetree")) - r = parse_path_one(field, &tmp.device_tree, p); + r = parse_path_one(tmp.path, line, field, &tmp.device_tree, p); else if (streq(field, "devicetree-overlay")) - r = parse_path_many(field, &tmp.device_tree_overlay, p); + r = parse_path_many(tmp.path, line, field, &tmp.device_tree_overlay, p); else { - log_notice("%s:%u: Unknown line \"%s\", ignoring.", tmp.path, line, field); + log_syntax(NULL, LOG_WARNING, tmp.path, line, 0, "Unknown line '%s', ignoring.", field); continue; } if (r < 0) - return log_error_errno(r, "%s:%u: Error while reading: %m", tmp.path, line); + return log_syntax(NULL, LOG_ERR, tmp.path, line, r, "Error while parsing: %m"); } *entry = tmp; @@ -325,9 +350,9 @@ int boot_loader_read_conf(BootConfig *config, FILE *file, const char *path) { if (r == 0) break; if (r == -ENOBUFS) - return log_error_errno(r, "%s:%u: Line too long", path, line); + return log_syntax(NULL, LOG_ERR, path, line, r, "Line too long."); if (r < 0) - return log_error_errno(r, "%s:%u: Error while reading: %m", path, line); + return log_syntax(NULL, LOG_ERR, path, line, r, "Error while reading: %m"); line++; @@ -337,11 +362,11 @@ int boot_loader_read_conf(BootConfig *config, FILE *file, const char *path) { r = extract_first_word(&p, &field, NULL, 0); if (r < 0) { - log_error_errno(r, "Failed to parse config file %s line %u: %m", path, line); + log_syntax(NULL, LOG_WARNING, path, line, r, "Failed to parse, ignoring line: %m"); continue; } if (r == 0) { - log_warning("%s:%u: Bad syntax", path, line); + log_syntax(NULL, LOG_WARNING, path, line, 0, "Bad syntax, ignoring line."); continue; } @@ -362,11 +387,11 @@ int boot_loader_read_conf(BootConfig *config, FILE *file, const char *path) { else if (streq(field, "beep")) r = free_and_strdup(&config->beep, p); else { - log_notice("%s:%u: Unknown line \"%s\", ignoring.", path, line, field); + log_syntax(NULL, LOG_WARNING, path, line, 0, "Unknown line '%s', ignoring.", field); continue; } if (r < 0) - return log_error_errno(r, "%s:%u: Error while reading: %m", path, line); + return log_syntax(NULL, LOG_ERR, path, line, r, "Error while parsing: %m"); } return 1; From 9ac2a89a8f02df9ea26df56891973172cf712d65 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 2 Sep 2022 11:33:48 +0200 Subject: [PATCH 4/8] bootspec: complain about loader.conf lines without parameter --- src/shared/bootspec.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index ebb172404b..3ca65454c1 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -369,6 +369,10 @@ int boot_loader_read_conf(BootConfig *config, FILE *file, const char *path) { log_syntax(NULL, LOG_WARNING, path, line, 0, "Bad syntax, ignoring line."); continue; } + if (isempty(p)) { + log_syntax(NULL, LOG_WARNING, path, line, 0, "Field '%s' without value, ignoring line.", field); + continue; + } if (streq(field, "default")) r = free_and_strdup(&config->default_pattern, p); From f70de82f9f5883ee35baccc70bd3120ebf397385 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 2 Sep 2022 11:40:26 +0200 Subject: [PATCH 5/8] bootspec: rename "id" function parameters to "fname" It's really the file name, so name it like that. Just some renaming. (This is preparation for later work, to extract tries-done/tries-left info from filenames) --- src/shared/bootspec.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 3ca65454c1..3ae05719a2 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -173,7 +173,7 @@ static int boot_entry_load_type1( FILE *f, const char *root, const char *dir, - const char *id, + const char *fname, BootEntry *entry) { _cleanup_(boot_entry_free) BootEntry tmp = { @@ -187,27 +187,27 @@ static int boot_entry_load_type1( assert(f); assert(root); assert(dir); - assert(id); + assert(fname); assert(entry); /* Loads a Type #1 boot menu entry from the specified FILE* object */ - if (!efi_loader_entry_name_valid(id)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid loader entry name: %s", id); + if (!efi_loader_entry_name_valid(fname)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid loader entry name: %s", fname); - c = endswith_no_case(id, ".conf"); + c = endswith_no_case(fname, ".conf"); if (!c) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid loader entry file suffix: %s", id); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid loader entry file suffix: %s", fname); - tmp.id = strdup(id); + tmp.id = strdup(fname); if (!tmp.id) return log_oom(); - tmp.id_old = strndup(id, c - id); /* Without .conf suffix */ + tmp.id_old = strndup(fname, c - fname); /* Without .conf suffix */ if (!tmp.id_old) return log_oom(); - tmp.path = path_join(dir, id); + tmp.path = path_join(dir, fname); if (!tmp.path) return log_oom(); @@ -291,19 +291,19 @@ int boot_config_load_type1( FILE *f, const char *root, const char *dir, - const char *id) { + const char *fname) { int r; assert(config); assert(f); assert(root); assert(dir); - assert(id); + assert(fname); if (!GREEDY_REALLOC0(config->entries, config->n_entries + 1)) return log_oom(); - r = boot_entry_load_type1(f, root, dir, id, config->entries + config->n_entries); + r = boot_entry_load_type1(f, root, dir, fname, config->entries + config->n_entries); if (r < 0) return r; From 7f5780edfbbeb29c7b5062194207580ce7e6ca51 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 2 Sep 2022 13:41:09 +0200 Subject: [PATCH 6/8] bootspec: properly parse tries done/tries left from bootspec file names This has been a long-time omission in the userspace parser of bootspec files. Correct that. Fixes: #16457 --- src/shared/bootspec.c | 147 ++++++++++++++++++++++++++++++++++----- src/shared/bootspec.h | 11 +++ src/test/test-bootspec.c | 53 ++++++++++++++ 3 files changed, 194 insertions(+), 17 deletions(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 3ae05719a2..5b7311f54d 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -169,6 +169,107 @@ static int parse_path_many( return strv_extend_strv(s, f, /* filter_duplicates= */ false); } +static int parse_tries(const char *fname, const char **p, unsigned *ret) { + _cleanup_free_ char *d = NULL; + unsigned tries; + size_t n; + int r; + + assert(fname); + assert(p); + assert(*p); + assert(ret); + + n = strspn(*p, DIGITS); + if (n == 0) { + *ret = UINT_MAX; + return 0; + } + + d = strndup(*p, n); + if (!d) + return log_oom(); + + r = safe_atou_full(d, 10, &tries); + if (r >= 0 && tries > INT_MAX) /* sd-boot allows INT_MAX, let's use the same limit */ + r = -ERANGE; + if (r < 0) + return log_error_errno(r, "Failed to parse tries counter of filename '%s': %m", fname); + + *p = *p + n; + *ret = tries; + return 1; +} + +int boot_filename_extract_tries( + const char *fname, + char **ret_stripped, + unsigned *ret_tries_left, + unsigned *ret_tries_done) { + + unsigned tries_left = UINT_MAX, tries_done = UINT_MAX; + _cleanup_free_ char *stripped = NULL; + const char *p, *suffix, *m; + int r; + + assert(fname); + assert(ret_stripped); + assert(ret_tries_left); + assert(ret_tries_done); + + /* Be liberal with suffix, only insist on a dot. After all we want to cover any capitalization here + * (vfat is case insensitive after all), and at least .efi and .conf as suffix. */ + suffix = strrchr(fname, '.'); + if (!suffix) + goto nothing; + + p = m = memrchr(fname, '+', suffix - fname); + if (!p) + goto nothing; + p++; + + r = parse_tries(fname, &p, &tries_left); + if (r < 0) + return r; + if (r == 0) + goto nothing; + + if (*p == '-') { + p++; + + r = parse_tries(fname, &p, &tries_done); + if (r < 0) + return r; + if (r == 0) + goto nothing; + } + + if (p != suffix) + goto nothing; + + stripped = strndup(fname, m - fname); + if (!stripped) + return log_oom(); + + if (!strextend(&stripped, suffix)) + return log_oom(); + + *ret_stripped = TAKE_PTR(stripped); + *ret_tries_left = tries_left; + *ret_tries_done = tries_done; + + return 0; + +nothing: + stripped = strdup(fname); + if (!stripped) + return log_oom(); + + *ret_stripped = TAKE_PTR(stripped); + *ret_tries_left = *ret_tries_done = UINT_MAX; + return 0; +} + static int boot_entry_load_type1( FILE *f, const char *root, @@ -176,10 +277,7 @@ static int boot_entry_load_type1( const char *fname, BootEntry *entry) { - _cleanup_(boot_entry_free) BootEntry tmp = { - .type = BOOT_ENTRY_CONF, - }; - + _cleanup_(boot_entry_free) BootEntry tmp = BOOT_ENTRY_INIT(BOOT_ENTRY_CONF); unsigned line = 1; char *c; int r; @@ -192,18 +290,18 @@ static int boot_entry_load_type1( /* Loads a Type #1 boot menu entry from the specified FILE* object */ - if (!efi_loader_entry_name_valid(fname)) + r = boot_filename_extract_tries(fname, &tmp.id, &tmp.tries_left, &tmp.tries_done); + if (r < 0) + return r; + + if (!efi_loader_entry_name_valid(tmp.id)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid loader entry name: %s", fname); - c = endswith_no_case(fname, ".conf"); + c = endswith_no_case(tmp.id, ".conf"); if (!c) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid loader entry file suffix: %s", fname); - tmp.id = strdup(fname); - if (!tmp.id) - return log_oom(); - - tmp.id_old = strndup(fname, c - fname); /* Without .conf suffix */ + tmp.id_old = strndup(tmp.id, c - tmp.id); /* Without .conf suffix */ if (!tmp.id_old) return log_oom(); @@ -559,11 +657,9 @@ static int boot_entry_load_unified( const char *cmdline, BootEntry *ret) { - _cleanup_free_ char *os_pretty_name = NULL, *os_image_id = NULL, *os_name = NULL, *os_id = NULL, + _cleanup_free_ char *fname = NULL, *os_pretty_name = NULL, *os_image_id = NULL, *os_name = NULL, *os_id = NULL, *os_image_version = NULL, *os_version = NULL, *os_version_id = NULL, *os_build_id = NULL; - _cleanup_(boot_entry_free) BootEntry tmp = { - .type = BOOT_ENTRY_UNIFIED, - }; + _cleanup_(boot_entry_free) BootEntry tmp = BOOT_ENTRY_INIT(BOOT_ENTRY_UNIFIED); const char *k, *good_name, *good_version, *good_sort_key; _cleanup_fclose_ FILE *f = NULL; int r; @@ -606,10 +702,14 @@ static int boot_entry_load_unified( &good_sort_key)) return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "Missing fields in os-release data from unified kernel image %s, refusing.", path); - r = path_extract_filename(path, &tmp.id); + r = path_extract_filename(path, &fname); if (r < 0) return log_error_errno(r, "Failed to extract file name from '%s': %m", path); + r = boot_filename_extract_tries(fname, &tmp.id, &tmp.tries_left, &tmp.tries_done); + if (r < 0) + return r; + if (!efi_loader_entry_name_valid(tmp.id)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid loader entry name: %s", tmp.id); @@ -1155,6 +1255,8 @@ int boot_config_augment_from_loader( .title = TAKE_PTR(t), .path = TAKE_PTR(p), .reported_by_loader = true, + .tries_left = UINT_MAX, + .tries_done = UINT_MAX, }; } @@ -1247,6 +1349,15 @@ int show_boot_entry( printf(" source: %s\n", link ?: e->path); } + if (e->tries_left != UINT_MAX) { + printf(" tries: %u left", e->tries_left); + + if (e->tries_done != UINT_MAX) + printf("; %u done\n", e->tries_done); + else + printf("\n"); + } + if (e->sort_key) printf(" sort-key: %s\n", e->sort_key); if (e->version) @@ -1325,7 +1436,9 @@ int show_boot_entries(const BootConfig *config, JsonFormatFlags json_format) { JSON_BUILD_PAIR_CONDITION(e->efi, "efi", JSON_BUILD_STRING(e->efi)), JSON_BUILD_PAIR_CONDITION(!strv_isempty(e->initrd), "initrd", JSON_BUILD_STRV(e->initrd)), JSON_BUILD_PAIR_CONDITION(e->device_tree, "devicetree", JSON_BUILD_STRING(e->device_tree)), - JSON_BUILD_PAIR_CONDITION(!strv_isempty(e->device_tree_overlay), "devicetreeOverlay", JSON_BUILD_STRV(e->device_tree_overlay)))); + JSON_BUILD_PAIR_CONDITION(!strv_isempty(e->device_tree_overlay), "devicetreeOverlay", JSON_BUILD_STRV(e->device_tree_overlay)), + JSON_BUILD_PAIR_CONDITION(e->tries_left != UINT_MAX, "triesLeft", JSON_BUILD_UNSIGNED(e->tries_left)), + JSON_BUILD_PAIR_CONDITION(e->tries_done != UINT_MAX, "triesDone", JSON_BUILD_UNSIGNED(e->tries_done)))); if (r < 0) return log_oom(); diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h index 36a3489e87..7f5d496b95 100644 --- a/src/shared/bootspec.h +++ b/src/shared/bootspec.h @@ -39,8 +39,17 @@ typedef struct BootEntry { char **initrd; char *device_tree; char **device_tree_overlay; + unsigned tries_left; + unsigned tries_done; } BootEntry; +#define BOOT_ENTRY_INIT(t) \ + { \ + .type = (t), \ + .tries_left = UINT_MAX, \ + .tries_done = UINT_MAX, \ + } + typedef struct BootConfig { char *default_pattern; char *timeout; @@ -116,3 +125,5 @@ int show_boot_entry( int show_boot_entries( const BootConfig *config, JsonFormatFlags json_format); + +int boot_filename_extract_tries(const char *fname, char **ret_stripped, unsigned *ret_tries_left, unsigned *ret_tries_done); diff --git a/src/test/test-bootspec.c b/src/test/test-bootspec.c index 46acd60e0d..3c645a3e79 100644 --- a/src/test/test-bootspec.c +++ b/src/test/test-bootspec.c @@ -93,4 +93,57 @@ TEST_RET(bootspec_sort) { return 0; } +static void test_extract_tries_one(const char *fname, int ret, const char *stripped, unsigned tries_left, unsigned tries_done) { + _cleanup_free_ char *p = NULL; + unsigned l = UINT_MAX, d = UINT_MAX; + + assert_se(boot_filename_extract_tries(fname, &p, &l, &d) == ret); + assert_se(streq_ptr(p, stripped)); + assert_se(l == tries_left); + assert_se(d == tries_done); +} + +TEST_RET(bootspec_extract_tries) { + test_extract_tries_one("foo.conf", 0, "foo.conf", UINT_MAX, UINT_MAX); + + test_extract_tries_one("foo+0.conf", 0, "foo.conf", 0, UINT_MAX); + test_extract_tries_one("foo+1.conf", 0, "foo.conf", 1, UINT_MAX); + test_extract_tries_one("foo+2.conf", 0, "foo.conf", 2, UINT_MAX); + test_extract_tries_one("foo+33.conf", 0, "foo.conf", 33, UINT_MAX); + + assert_cc(INT_MAX == INT32_MAX); + test_extract_tries_one("foo+2147483647.conf", 0, "foo.conf", 2147483647, UINT_MAX); + test_extract_tries_one("foo+2147483648.conf", -ERANGE, NULL, UINT_MAX, UINT_MAX); + + test_extract_tries_one("foo+33-0.conf", 0, "foo.conf", 33, 0); + test_extract_tries_one("foo+33-1.conf", 0, "foo.conf", 33, 1); + test_extract_tries_one("foo+33-107.conf", 0, "foo.conf", 33, 107); + test_extract_tries_one("foo+33-107.efi", 0, "foo.efi", 33, 107); + test_extract_tries_one("foo+33-2147483647.conf", 0, "foo.conf", 33, 2147483647); + test_extract_tries_one("foo+33-2147483648.conf", -ERANGE, NULL, UINT_MAX, UINT_MAX); + + test_extract_tries_one("foo+007-000008.conf", 0, "foo.conf", 7, 8); + + test_extract_tries_one("foo-1.conf", 0, "foo-1.conf", UINT_MAX, UINT_MAX); + test_extract_tries_one("foo-999.conf", 0, "foo-999.conf", UINT_MAX, UINT_MAX); + test_extract_tries_one("foo-.conf", 0, "foo-.conf", UINT_MAX, UINT_MAX); + + test_extract_tries_one("foo+.conf", 0, "foo+.conf", UINT_MAX, UINT_MAX); + test_extract_tries_one("+.conf", 0, "+.conf", UINT_MAX, UINT_MAX); + test_extract_tries_one("-.conf", 0, "-.conf", UINT_MAX, UINT_MAX); + test_extract_tries_one("", 0, "", UINT_MAX, UINT_MAX); + + test_extract_tries_one("+1.", 0, ".", 1, UINT_MAX); + test_extract_tries_one("+1-7.", 0, ".", 1, 7); + + test_extract_tries_one("some+name+24324-22.efi", 0, "some+name.efi", 24324, 22); + test_extract_tries_one("sels+2-3+7-6.", 0, "sels+2-3.", 7, 6); + test_extract_tries_one("a+1-2..", 0, "a+1-2..", UINT_MAX, UINT_MAX); + test_extract_tries_one("ses.sgesge.+4-1.efi", 0, "ses.sgesge..efi", 4, 1); + test_extract_tries_one("abc+0x4.conf", 0, "abc+0x4.conf", UINT_MAX, UINT_MAX); + test_extract_tries_one("def+1-0x3.conf", 0, "def+1-0x3.conf", UINT_MAX, UINT_MAX); + + return 0; +} + DEFINE_TEST_MAIN(LOG_INFO); From 293e224056e07eb9c48042228ddb27404c8e518b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 2 Sep 2022 13:41:59 +0200 Subject: [PATCH 7/8] bootspec: clarify we knowingly ignore all errors but ENOMEM --- src/shared/bootspec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 5b7311f54d..332b5cfed3 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -643,7 +643,7 @@ static int boot_entries_find_type1( continue; r = boot_config_load_type1(config, f, root, dir, de->d_name); - if (r == -ENOMEM) + if (r == -ENOMEM) /* ignore all other errors */ return r; } From 95150f3f560f583effade9804289e371f47acfad Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 2 Sep 2022 13:48:32 +0200 Subject: [PATCH 8/8] update TODO --- TODO | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/TODO b/TODO index 35bae243f3..3307ce1992 100644 --- a/TODO +++ b/TODO @@ -597,10 +597,9 @@ Features: * doc: prep a document explaining PID 1's internal logic, i.e. transactions, jobs, units -* bootspec: remove tries counter from boot entry ids - * bootspec: bring UEFI and userspace enumeration of bootspec entries back into - sync, i.e. parse out tries in both + sync, i.e. parse out architecture field in sd-boot (currently only done in + userspace) * automatically ignore threaded cgroups in cg_xyz(). @@ -1594,14 +1593,6 @@ Features: * firstboot: make it useful to be run immediately after yum --installroot to set up a machine. (most specifically, make --copy-root-password work even if /etc/passwd already exists -* sd-boot: define a drop-in dir in the ESP that may contain X.509 - certificates. If the firmware is detected to be in setup mode, automatically - enroll them as PK/KEK/db, turn off setup mode and proceed. Optionally, - instead of auto-enrolling them add them to the sd-boot menu, giving the user - the option to manually enroll them, after selecting the menu entry. This way, - installer images can just drop the certfiicates in the ESP, and on first boot - can easily enroll the keys without ever booting up. - * efi stub: optionally, load initrd from disk as a separate file, HMAC check it with key from TPM, bound to PCR, refusing if failing. This would then allow traditional distros that generate initrds locally to secure them with TPM: @@ -1623,7 +1614,6 @@ Features: - show whether UEFI audit mode is available - teach it to prepare an ESP wholesale, i.e. with mkfs.vfat invocation - teach it to copy in unified kernel images and maybe type #1 boot loader spec entries from host - - bootspec: properly support boot attempt counters when parsing entry file names * kernel-install: - optionally, support generating type #2 entries instead of type #1, including signing them