From: Emanuele Giuseppe Esposito Date: Thu, 15 Feb 2024 14:25:15 +0000 (-0500) Subject: bootctl: additional fixes for local/global UKI PE addons X-Git-Tag: v256-rc1~600 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=706ca67d3074b2a405ee8fe5de307416e4915b9f;p=thirdparty%2Fsystemd.git bootctl: additional fixes for local/global UKI PE addons Fix various memory leaks and names used in https://github.com/systemd/systemd/pull/28761. --- diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 9a1f5ab127d..42b52c88453 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -43,15 +43,6 @@ static const char* const boot_entry_type_json_table[_BOOT_ENTRY_TYPE_MAX] = { DEFINE_STRING_TABLE_LOOKUP_TO_STRING(boot_entry_type_json, BootEntryType); -BootEntryAddon* boot_entry_addon_free(BootEntryAddon *addon) { - if (!addon) - return NULL; - - free(addon->location); - free(addon->cmdline); - return mfree(addon); -} - static void boot_entry_free(BootEntry *entry) { assert(entry); @@ -783,7 +774,10 @@ static int find_cmdline_section( char **ret_cmdline) { int r; - char *cmdline = NULL; + char *cmdline = NULL, *t = NULL; + _cleanup_free_ char *word = NULL; + + assert(path); if (!ret_cmdline) return 0; @@ -793,7 +787,16 @@ static int find_cmdline_section( *ret_cmdline = NULL; else if (r < 0) return log_warning_errno(r, "Failed to read .cmdline section of '%s': %m", path); - else if (ret_cmdline) + + word = strdup(cmdline); + if (!word) + return log_oom(); + + /* Quick test to check if there is actual content in the addon cmdline */ + t = delete_chars(word, NULL); + if (t[0] == 0) + *ret_cmdline = NULL; + else *ret_cmdline = TAKE_PTR(cmdline); return 0; @@ -859,7 +862,15 @@ static int find_addon_sections( if (r < 0) return r; - return find_cmdline_section(fd, path, sections, pe_header, ret_cmdline); + r = find_cmdline_section(fd, path, sections, pe_header, ret_cmdline); + /* If addon cmdline is empty or contains just separators, + * don't bother tracking it. + * Don't check r because it cannot return <0 if cmdline is empty, + * as cmdline is always optional. */ + if (!ret_cmdline) + return log_warning_errno(SYNTHETIC_ERRNO(ENOENT), "Addon %s contains empty cmdline and will be therefore ignored.", path); + + return r; } static int insert_boot_entry_addon( @@ -867,30 +878,40 @@ static int insert_boot_entry_addon( char *location, char *cmdline) { - if (!GREEDY_REALLOC(addons->items, addons->count + 1)) + if (!GREEDY_REALLOC(addons->items, addons->n_items + 1)) return log_oom(); - addons->items[addons->count] = (BootEntryAddon) { + addons->items[addons->n_items] = (BootEntryAddon) { .location = location, .cmdline = cmdline, }; - addons->count++; + addons->n_items++; return 0; } +static void boot_entry_addons_done(BootEntryAddons *addons) { + FOREACH_ARRAY(addon, addons->items, addons->n_items) { + free(addon->cmdline); + free(addon->location); + } + addons->items = mfree(addons->items); + addons->n_items = 0; +} + static int boot_entries_find_unified_addons( BootConfig *config, int d_fd, const char *addon_dir, const char *root, - BootEntryAddons *addons) { + BootEntryAddons *ret_addons) { _cleanup_closedir_ DIR *d = NULL; _cleanup_free_ char *full = NULL; + _cleanup_(boot_entry_addons_done) BootEntryAddons addons = {}; int r; - assert(addons); + assert(ret_addons); assert(config); r = chase_and_opendirat(d_fd, addon_dir, CHASE_AT_RESOLVE_IN_ROOT, &full, &d); @@ -899,8 +920,6 @@ static int boot_entries_find_unified_addons( if (r < 0) return log_error_errno(r, "Failed to open '%s/%s': %m", root, addon_dir); - *addons = (BootEntryAddons) {}; - FOREACH_DIRENT(de, d, return log_error_errno(errno, "Failed to read %s: %m", full)) { _cleanup_free_ char *j = NULL, *cmdline = NULL, *location = NULL; _cleanup_close_ int fd = -EBADF; @@ -934,12 +953,15 @@ static int boot_entries_find_unified_addons( if (!location) return log_oom(); - r = insert_boot_entry_addon(addons, TAKE_PTR(location), TAKE_PTR(cmdline)); - if (r < 0) { - free(addons); + r = insert_boot_entry_addon(&addons, location, cmdline); + if (r < 0) return r; - } + + TAKE_PTR(location); + TAKE_PTR(cmdline); } + + *ret_addons = TAKE_STRUCT(addons); return 0; } @@ -1439,52 +1461,74 @@ static void print_addon( const char *addon_str) { printf(" %s: %s\n", addon_str, addon->location); - printf(" cmdline: %s%s\n", special_glyph(SPECIAL_GLYPH_TREE_RIGHT), addon->cmdline); + printf(" options: %s%s\n", special_glyph(SPECIAL_GLYPH_TREE_RIGHT), addon->cmdline); +} + +static int indent_embedded_newlines(char *cmdline, char **ret_cmdline) { + _cleanup_free_ char *t = NULL; + _cleanup_strv_free_ char **ts = NULL; + + assert(ret_cmdline); + + ts = strv_split_newlines(cmdline); + if (!ts) + return -ENOMEM; + + t = strv_join(ts, "\n "); + if (!t) + return -ENOMEM; + + *ret_cmdline = TAKE_PTR(t); + + return 0; } static int print_cmdline( const BootEntry *e, const BootEntryAddons *global_arr) { - _cleanup_free_ char *final_cmdline = NULL; + _cleanup_free_ char *options = NULL, *combined_cmdline = NULL, *t2 = NULL; assert(e); if (!strv_isempty(e->options)) { - _cleanup_free_ char *t = NULL, *t2 = NULL; - _cleanup_strv_free_ char **ts = NULL; + _cleanup_free_ char *t = NULL; - t = strv_join(e->options, " "); - if (!t) + options = strv_join(e->options, " "); + if (!options) return log_oom(); - ts = strv_split_newlines(t); - if (!ts) + if (indent_embedded_newlines(options, &t) < 0) return log_oom(); - t2 = strv_join(ts, "\n "); + printf(" options: %s\n", t); + t2 = strdup(options); if (!t2) return log_oom(); - - printf(" ukiCmdline: %s\n", t2); - final_cmdline = TAKE_PTR(t2); } - FOREACH_ARRAY(addon, global_arr->items, global_arr->count) { - print_addon(addon, "globalAddon"); - if (!strextend(&final_cmdline, " ", addon->cmdline)) + FOREACH_ARRAY(addon, global_arr->items, global_arr->n_items) { + print_addon(addon, "global-addon"); + if (!strextend(&t2, " ", addon->cmdline)) return log_oom(); } - FOREACH_ARRAY(addon, e->local_addons.items, e->local_addons.count) { + FOREACH_ARRAY(addon, e->local_addons.items, e->local_addons.n_items) { /* Add space at the beginning of addon_str to align it correctly */ - print_addon(addon, " localAddon"); - if (!strextend(&final_cmdline, " ", addon->cmdline)) + print_addon(addon, " local-addon"); + if (!strextend(&t2, " ", addon->cmdline)) return log_oom(); } - if (final_cmdline) - printf(" options: %s\n", final_cmdline); + /* Don't print the combined cmdline if it's same as options. */ + if (streq_ptr(t2, options)) + return 0; + + if (indent_embedded_newlines(t2, &combined_cmdline) < 0) + return log_oom(); + + if (combined_cmdline) + printf(" cmdline: %s\n", combined_cmdline); return 0; } @@ -1499,7 +1543,7 @@ static int json_addon( r = json_variant_append_arrayb(array, JSON_BUILD_OBJECT( JSON_BUILD_PAIR(addon_str, JSON_BUILD_STRING(addon->location)), - JSON_BUILD_PAIR("cmdline", JSON_BUILD_STRING(addon->cmdline)))); + JSON_BUILD_PAIR("options", JSON_BUILD_STRING(addon->cmdline)))); if (r < 0) return log_oom(); @@ -1509,42 +1553,41 @@ static int json_addon( static int json_cmdline( const BootEntry *e, const BootEntryAddons *global_arr, + const char *def_cmdline, JsonVariant **v) { - _cleanup_free_ char *final_cmdline = NULL, *def_cmdline = NULL; + _cleanup_free_ char *combined_cmdline = NULL; _cleanup_(json_variant_unrefp) JsonVariant *addons_array = NULL; int r; assert(e); - if (!strv_isempty(e->options)) { - def_cmdline = strv_join(e->options, " "); - if (!def_cmdline) + if (def_cmdline) { + combined_cmdline = strdup(def_cmdline); + if (!combined_cmdline) return log_oom(); - final_cmdline = TAKE_PTR(def_cmdline); } - FOREACH_ARRAY(addon, global_arr->items, global_arr->count) { + FOREACH_ARRAY(addon, global_arr->items, global_arr->n_items) { r = json_addon(addon, "globalAddon", &addons_array); if (r < 0) return r; - if (!strextend(&final_cmdline, " ", addon->cmdline)) + if (!strextend(&combined_cmdline, " ", addon->cmdline)) return log_oom(); } - FOREACH_ARRAY(addon, e->local_addons.items, e->local_addons.count) { + FOREACH_ARRAY(addon, e->local_addons.items, e->local_addons.n_items) { r = json_addon(addon, "localAddon", &addons_array); if (r < 0) return r; - if (!strextend(&final_cmdline, " ", addon->cmdline)) + if (!strextend(&combined_cmdline, " ", addon->cmdline)) return log_oom(); } r = json_variant_merge_objectb( v, JSON_BUILD_OBJECT( - JSON_BUILD_PAIR_CONDITION(def_cmdline, "ukiCmdline", JSON_BUILD_STRING(def_cmdline)), JSON_BUILD_PAIR("addons", JSON_BUILD_VARIANT(addons_array)), - JSON_BUILD_PAIR_CONDITION(final_cmdline, "options", JSON_BUILD_STRING(final_cmdline)))); + JSON_BUILD_PAIR_CONDITION(combined_cmdline, "cmdline", JSON_BUILD_STRING(combined_cmdline)))); if (r < 0) return log_oom(); return 0; @@ -1654,6 +1697,7 @@ int show_boot_entry( int boot_entry_to_json(const BootConfig *c, size_t i, JsonVariant **ret) { _cleanup_(json_variant_unrefp) JsonVariant *v = NULL; + _cleanup_free_ char *opts = NULL; const BootEntry *e; int r; @@ -1667,6 +1711,12 @@ int boot_entry_to_json(const BootConfig *c, size_t i, JsonVariant **ret) { e = c->entries + i; + if (!strv_isempty(e->options)) { + opts = strv_join(e->options, " "); + if (!opts) + return log_oom(); + } + r = json_variant_merge_objectb( &v, JSON_BUILD_OBJECT( JSON_BUILD_PAIR("type", JSON_BUILD_STRING(boot_entry_type_json_to_string(e->type))), @@ -1679,6 +1729,7 @@ int boot_entry_to_json(const BootConfig *c, size_t i, JsonVariant **ret) { JSON_BUILD_PAIR_CONDITION(e->version, "version", JSON_BUILD_STRING(e->version)), JSON_BUILD_PAIR_CONDITION(e->machine_id, "machineId", JSON_BUILD_STRING(e->machine_id)), JSON_BUILD_PAIR_CONDITION(e->architecture, "architecture", JSON_BUILD_STRING(e->architecture)), + JSON_BUILD_PAIR_CONDITION(opts, "options", JSON_BUILD_STRING(opts)), JSON_BUILD_PAIR_CONDITION(e->kernel, "linux", JSON_BUILD_STRING(e->kernel)), 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)), @@ -1701,7 +1752,7 @@ int boot_entry_to_json(const BootConfig *c, size_t i, JsonVariant **ret) { if (r < 0) return log_oom(); - r = json_cmdline(e, &c->global_addons, &v); + r = json_cmdline(e, &c->global_addons, opts, &v); if (r < 0) return log_oom(); diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h index 5f9a0d70562..1885a88a508 100644 --- a/src/shared/bootspec.h +++ b/src/shared/bootspec.h @@ -27,7 +27,7 @@ typedef struct BootEntryAddon { typedef struct BootEntryAddons { BootEntryAddon *items; - size_t count; + size_t n_items; } BootEntryAddons; BootEntryAddon* boot_entry_addon_free(BootEntryAddon *t);