diff --git a/docs/COREDUMP_PACKAGE_METADATA.md b/docs/COREDUMP_PACKAGE_METADATA.md index 54ff31ee78..bf7588f43a 100644 --- a/docs/COREDUMP_PACKAGE_METADATA.md +++ b/docs/COREDUMP_PACKAGE_METADATA.md @@ -52,12 +52,12 @@ Value: a JSON string with the structure described below ```json { - "packageType":"rpm", # this provides a namespace for the package+package-version fields - "packageDistro":"fedora", - "packageDistroVersion":"33", - "package":"coreutils", - "packageVersion": "4711.0815.fc13.arm32", - "cpe": # A CPE name for the operating system, `CPE_NAME` from os-release is a good default + "type":"rpm", # this provides a namespace for the package+package-version fields + "os":"fedora", + "osVersion":"33", + "name":"coreutils", + "version": "4711.0815.fc13.arm32", + "osCpe": # A CPE name for the operating system, `CPE_NAME` from os-release is a good default } ``` @@ -72,38 +72,34 @@ SECTIONS { .note.package : ALIGN(4) { BYTE(0x04) BYTE(0x00) BYTE(0x00) BYTE(0x00) /* Length of Owner including NUL */ - BYTE(0x73) BYTE(0x00) BYTE(0x00) BYTE(0x00) /* Length of Value including NUL */ + BYTE(0x64) BYTE(0x00) BYTE(0x00) BYTE(0x00) /* Length of Value including NUL */ BYTE(0x7e) BYTE(0x1a) BYTE(0xfe) BYTE(0xca) /* Note ID */ BYTE(0x46) BYTE(0x44) BYTE(0x4f) BYTE(0x00) /* Owner: 'FDO\x00' */ - BYTE(0x7b) BYTE(0x22) BYTE(0x70) BYTE(0x61) /* Value: '{"packageType":"rpm","package":"systemd","packageVersion":"248~rc2-1.fc34","cpe":"cpe:/o:fedoraproject:fedora:33"}\x00\x00' */ - BYTE(0x63) BYTE(0x6b) BYTE(0x61) BYTE(0x67) - BYTE(0x65) BYTE(0x54) BYTE(0x79) BYTE(0x70) - BYTE(0x65) BYTE(0x22) BYTE(0x3a) BYTE(0x22) - BYTE(0x72) BYTE(0x70) BYTE(0x6d) BYTE(0x22) - BYTE(0x2c) BYTE(0x22) BYTE(0x70) BYTE(0x61) - BYTE(0x63) BYTE(0x6b) BYTE(0x61) BYTE(0x67) - BYTE(0x65) BYTE(0x22) BYTE(0x3a) BYTE(0x22) - BYTE(0x73) BYTE(0x79) BYTE(0x73) BYTE(0x74) - BYTE(0x65) BYTE(0x6d) BYTE(0x64) BYTE(0x22) - BYTE(0x2c) BYTE(0x22) BYTE(0x70) BYTE(0x61) - BYTE(0x63) BYTE(0x6b) BYTE(0x61) BYTE(0x67) - BYTE(0x65) BYTE(0x56) BYTE(0x65) BYTE(0x72) - BYTE(0x73) BYTE(0x69) BYTE(0x6f) BYTE(0x6e) - BYTE(0x22) BYTE(0x3a) BYTE(0x22) BYTE(0x32) - BYTE(0x34) BYTE(0x38) BYTE(0x7e) BYTE(0x72) - BYTE(0x63) BYTE(0x32) BYTE(0x2d) BYTE(0x31) - BYTE(0x2e) BYTE(0x66) BYTE(0x63) BYTE(0x33) - BYTE(0x34) BYTE(0x22) BYTE(0x2c) BYTE(0x22) - BYTE(0x63) BYTE(0x70) BYTE(0x65) BYTE(0x22) - BYTE(0x3a) BYTE(0x22) BYTE(0x63) BYTE(0x70) - BYTE(0x65) BYTE(0x3a) BYTE(0x2f) BYTE(0x6f) - BYTE(0x3a) BYTE(0x66) BYTE(0x65) BYTE(0x64) - BYTE(0x6f) BYTE(0x72) BYTE(0x61) BYTE(0x70) - BYTE(0x72) BYTE(0x6f) BYTE(0x6a) BYTE(0x65) - BYTE(0x63) BYTE(0x74) BYTE(0x3a) BYTE(0x66) - BYTE(0x65) BYTE(0x64) BYTE(0x6f) BYTE(0x72) - BYTE(0x61) BYTE(0x3a) BYTE(0x33) BYTE(0x33) - BYTE(0x22) BYTE(0x7d) BYTE(0x00) BYTE(0x00) + BYTE(0x7b) BYTE(0x22) BYTE(0x74) BYTE(0x79) /* Value: '{"type":"rpm","name":"systemd","version":"248~rc2-1.fc34","osCpe":"cpe:/o:fedoraproject:fedora:33"}\x00' */ + BYTE(0x70) BYTE(0x65) BYTE(0x22) BYTE(0x3a) + BYTE(0x22) BYTE(0x72) BYTE(0x70) BYTE(0x6d) + BYTE(0x22) BYTE(0x2c) BYTE(0x22) BYTE(0x6e) + BYTE(0x61) BYTE(0x6d) BYTE(0x65) BYTE(0x22) + BYTE(0x3a) BYTE(0x22) BYTE(0x73) BYTE(0x79) + BYTE(0x73) BYTE(0x74) BYTE(0x65) BYTE(0x6d) + BYTE(0x64) BYTE(0x22) BYTE(0x2c) BYTE(0x22) + BYTE(0x76) BYTE(0x65) BYTE(0x72) BYTE(0x73) + BYTE(0x69) BYTE(0x6f) BYTE(0x6e) BYTE(0x22) + BYTE(0x3a) BYTE(0x22) BYTE(0x32) BYTE(0x34) + BYTE(0x38) BYTE(0x7e) BYTE(0x72) BYTE(0x63) + BYTE(0x32) BYTE(0x2d) BYTE(0x31) BYTE(0x2e) + BYTE(0x66) BYTE(0x63) BYTE(0x33) BYTE(0x34) + BYTE(0x22) BYTE(0x2c) BYTE(0x22) BYTE(0x6f) + BYTE(0x73) BYTE(0x43) BYTE(0x70) BYTE(0x65) + BYTE(0x22) BYTE(0x3a) BYTE(0x22) BYTE(0x63) + BYTE(0x70) BYTE(0x65) BYTE(0x3a) BYTE(0x2f) + BYTE(0x6f) BYTE(0x3a) BYTE(0x66) BYTE(0x65) + BYTE(0x64) BYTE(0x6f) BYTE(0x72) BYTE(0x61) + BYTE(0x70) BYTE(0x72) BYTE(0x6f) BYTE(0x6a) + BYTE(0x65) BYTE(0x63) BYTE(0x74) BYTE(0x3a) + BYTE(0x66) BYTE(0x65) BYTE(0x64) BYTE(0x6f) + BYTE(0x72) BYTE(0x61) BYTE(0x3a) BYTE(0x33) + BYTE(0x33) BYTE(0x22) BYTE(0x7d) BYTE(0x00) } } INSERT AFTER .note.gnu.build-id; diff --git a/man/systemd-coredump.xml b/man/systemd-coredump.xml index d994a21d9b..707905f1ba 100644 --- a/man/systemd-coredump.xml +++ b/man/systemd-coredump.xml @@ -353,9 +353,9 @@ flags: ... - COREDUMP_PKGMETA_PACKAGE= - COREDUMP_PKGMETA_PACKAGEVERSION= - COREDUMP_PKGMETA_JSON= + COREDUMP_PACKAGE_NAME= + COREDUMP_PACKAGE_VERSION= + COREDUMP_PACKAGE_JSON= If the executable contained .package metadata ELF notes, they will be parsed and attached. The package and packageVersion diff --git a/src/basic/path-util.c b/src/basic/path-util.c index f40f3f27e9..f98859939c 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -527,6 +527,27 @@ bool path_equal_or_files_same(const char *a, const char *b, int flags) { return path_equal(a, b) || files_same(a, b, flags) > 0; } +bool path_equal_filename(const char *a, const char *b) { + _cleanup_free_ char *a_basename = NULL, *b_basename = NULL; + int r; + + assert(a); + assert(b); + + r = path_extract_filename(a, &a_basename); + if (r < 0) { + log_debug_errno(r, "Failed to parse basename of %s: %m", a); + return false; + } + r = path_extract_filename(b, &b_basename); + if (r < 0) { + log_debug_errno(r, "Failed to parse basename of %s: %m", b); + return false; + } + + return path_equal(a_basename, b_basename); +} + char* path_join_internal(const char *first, ...) { char *joined, *q; const char *p; diff --git a/src/basic/path-util.h b/src/basic/path-util.h index c0746f68d7..f82d935dc5 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -61,6 +61,8 @@ char* path_startswith(const char *path, const char *prefix) _pure_; int path_compare(const char *a, const char *b) _pure_; bool path_equal(const char *a, const char *b) _pure_; bool path_equal_or_files_same(const char *a, const char *b, int flags); +/* Compares only the last portion of the input paths, ie: the filenames */ +bool path_equal_filename(const char *a, const char *b); char* path_join_internal(const char *first, ...); #define path_join(x, ...) path_join_internal(x, __VA_ARGS__, POINTER_MAX) diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index be813f57f1..b6cc7e3887 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -792,56 +792,23 @@ log: if (r < 0) return log_error_errno(r, "Failed to format JSON package metadata: %m"); - (void) iovw_put_string_field(iovw, "COREDUMP_PKGMETA_JSON=", formatted_json); + (void) iovw_put_string_field(iovw, "COREDUMP_PACKAGE_JSON=", formatted_json); } JSON_VARIANT_OBJECT_FOREACH(module_name, module_json, json_metadata) { - _cleanup_free_ char *module_basename = NULL, *exe_basename = NULL; - const char *key; - JsonVariant *w; - - /* The module name, most likely parsed from the ELF core file, - * sometimes contains the full path and sometimes does not. */ - r = path_extract_filename(module_name, &module_basename); - if (r < 0) - return log_error_errno(r, "Failed to parse module basename: %m"); - r = path_extract_filename(context->meta[META_EXE], &exe_basename); - if (r < 0) - return log_error_errno(r, "Failed to parse executable basename: %m"); + JsonVariant *package_name, *package_version; /* We only add structured fields for the 'main' ELF module */ - if (!streq(module_basename, exe_basename)) + if (!path_equal_filename(module_name, context->meta[META_EXE])) continue; - /* Cannot nest two JSON_VARIANT_OBJECT_FOREACH as they define the same - * iterator variable '_state' */ - for (struct json_variant_foreach_state _state2 = { (module_json), 0 }; \ - json_variant_is_object(_state2.variant) && \ - _state2.idx < json_variant_elements(_state2.variant) && \ - ({ key = json_variant_string(json_variant_by_index(_state2.variant, _state2.idx)); \ - w = json_variant_by_index(_state2.variant, _state2.idx + 1); \ - true; }); \ - _state2.idx += 2) { - _cleanup_free_ char *metadata_id = NULL, *key_upper = NULL; + package_name = json_variant_by_key(module_json, "name"); + if (package_name) + (void) iovw_put_string_field(iovw, "COREDUMP_PACKAGE_NAME=", json_variant_string(package_name)); - if (!json_variant_is_string(w)) - continue; - - if (!STR_IN_SET(key, "package", "packageVersion")) - continue; - - /* Journal metadata field names need to be upper case */ - key_upper = strdup(key); - if (!key_upper) - return log_oom(); - key_upper = ascii_strupper(key_upper); - - metadata_id = strjoin("COREDUMP_PKGMETA_", key_upper, "="); - if (!metadata_id) - return log_oom(); - - (void) iovw_put_string_field(iovw, metadata_id, json_variant_string(w)); - } + package_version = json_variant_by_key(module_json, "version"); + if (package_version) + (void) iovw_put_string_field(iovw, "COREDUMP_PACKAGE_VERSION=", json_variant_string(package_version)); } /* Optionally store the entire coredump in the journal */ diff --git a/src/coredump/coredumpctl.c b/src/coredump/coredumpctl.c index 3418753a31..02bad966c1 100644 --- a/src/coredump/coredumpctl.c +++ b/src/coredump/coredumpctl.c @@ -575,9 +575,9 @@ static int print_info(FILE *file, sd_journal *j, bool need_space) { RETRIEVE(d, l, "COREDUMP_FILENAME", filename); RETRIEVE(d, l, "COREDUMP_TRUNCATED", truncated); RETRIEVE(d, l, "COREDUMP", coredump); - RETRIEVE(d, l, "COREDUMP_PKGMETA_PACKAGE", pkgmeta_name); - RETRIEVE(d, l, "COREDUMP_PKGMETA_PACKAGEVERSION", pkgmeta_version); - RETRIEVE(d, l, "COREDUMP_PKGMETA_JSON", pkgmeta_json); + RETRIEVE(d, l, "COREDUMP_PACKAGE_NAME", pkgmeta_name); + RETRIEVE(d, l, "COREDUMP_PACKAGE_VERSION", pkgmeta_version); + RETRIEVE(d, l, "COREDUMP_PACKAGE_JSON", pkgmeta_json); RETRIEVE(d, l, "_BOOT_ID", boot_id); RETRIEVE(d, l, "_MACHINE_ID", machine_id); RETRIEVE(d, l, "MESSAGE", message); @@ -729,53 +729,22 @@ static int print_info(FILE *file, sd_journal *j, bool need_space) { _cleanup_(json_variant_unrefp) JsonVariant *v = NULL; r = json_parse(pkgmeta_json, 0, &v, NULL, NULL); - if (r < 0) { + if (r < 0) log_warning_errno(r, "json_parse on %s failed, ignoring: %m", pkgmeta_json); - } else { + else { const char *module_name; JsonVariant *module_json; - /* Cannot nest two JSON_VARIANT_OBJECT_FOREACH as they define the same - * iterator variable '_state' */ - for (struct json_variant_foreach_state _state2 = { (v), 0 }; \ - json_variant_is_object(_state2.variant) && \ - _state2.idx < json_variant_elements(_state2.variant) && \ - ({ module_name = json_variant_string(json_variant_by_index(_state2.variant, _state2.idx)); \ - module_json = json_variant_by_index(_state2.variant, _state2.idx + 1); \ - true; }); \ - _state2.idx += 2) { - _cleanup_free_ char *module_basename = NULL, *exe_basename = NULL; - const char *key; - JsonVariant *w; - - /* The module name, most likely parsed from the ELF core file, - * sometimes contains the full path and sometimes does not. */ - r = path_extract_filename(module_name, &module_basename); - if (r < 0) { - log_warning_errno(r, "Failed to parse module basename: %m"); - break; - } - r = path_extract_filename(exe, &exe_basename); - if (r < 0) { - log_warning_errno(r, "Failed to parse executable basename: %m"); - break; - } + JSON_VARIANT_OBJECT_FOREACH(module_name, module_json, v) { + JsonVariant *build_id; /* We only print the build-id for the 'main' ELF module */ - if (!streq(module_basename, exe_basename)) + if (!path_equal_filename(module_name, exe)) continue; - JSON_VARIANT_OBJECT_FOREACH(key, w, module_json) { - if (!json_variant_is_string(w)) - continue; - - if (!streq(key, "buildid")) - continue; - - fprintf(file, " build-id: %s\n", json_variant_string(w)); - - break; - } + build_id = json_variant_by_key(module_json, "buildId"); + if (build_id) + fprintf(file, " build-id: %s\n", json_variant_string(build_id)); break; } diff --git a/src/coredump/stacktrace.c b/src/coredump/stacktrace.c index c16f12f09b..102ad2e65b 100644 --- a/src/coredump/stacktrace.c +++ b/src/coredump/stacktrace.c @@ -166,7 +166,6 @@ static int parse_package_metadata(const char *name, JsonVariant *id_json, Elf *e * magic ID is always the same. */ if (note_header.n_type == ELF_PACKAGE_METADATA_ID) { _cleanup_(json_variant_unrefp) JsonVariant *v = NULL, *w = NULL; - char *name_key = NULL; r = json_parse(payload, 0, &v, NULL, NULL); if (r < 0) { @@ -182,7 +181,7 @@ static int parse_package_metadata(const char *name, JsonVariant *id_json, Elf *e fputc('\n', c->f); /* Secondly, if we have a build-id, merge it in the same JSON object - * so that it apperas all nicely together in the logs/metadata. */ + * so that it appears all nicely together in the logs/metadata. */ if (id_json) { r = json_variant_merge(&v, id_json); if (r < 0) { @@ -206,14 +205,9 @@ static int parse_package_metadata(const char *name, JsonVariant *id_json, Elf *e } /* Finally stash the name, so we avoid double visits. */ - name_key = strdup(name); - if (!name_key) { - log_oom(); - return DWARF_CB_ABORT; - } - r = set_ensure_consume(c->modules, &string_hash_ops, name_key); + r = set_put_strdup(c->modules, name); if (r < 0) { - log_error_errno(r, "set_ensure_consume failed: %m"); + log_error_errno(r, "set_put_strdup failed: %m"); return DWARF_CB_ABORT; } @@ -247,35 +241,26 @@ static int module_callback(Dwfl_Module *mod, void **userdata, const char *name, * The build-id is easy, as libdwfl parses it during the dwfl_core_file_report() call and * stores it separately in an internal library struct. */ id_len = dwfl_module_build_id(mod, &id, &id_vaddr); - if (id_len <= 0) { + if (id_len <= 0) /* If we don't find a build-id, note it in the journal message, and try * anyway to find the package metadata. It's unlikely to have the latter * without the former, but there's no hard rule. */ - fprintf(c->f, "Found module %s without build-id\n", name); - } else { - _cleanup_free_ char *id_hex = NULL, *id_hex_prefixed = NULL; - - id_hex = hexmem(id, id_len); - if (!id_hex) { - log_oom(); - return DWARF_CB_ABORT; - } - - fprintf(c->f, "Found module %s with build-id: %s\n", name, id_hex); + fprintf(c->f, "Found module %s without build-id.\n", name); + else { + JsonVariant *build_id; /* We will later parse package metadata json and pass it to our caller. Prepare the * build-id in json format too, so that it can be appended and parsed cleanly. It * will then be added as metadata to the journal message with the stack trace. */ - id_hex_prefixed = strjoin("{\"buildid\":\"", id_hex, "\"}"); - if (!id_hex_prefixed) { - log_oom(); - return DWARF_CB_ABORT; - } - r = json_parse(id_hex_prefixed, 0, &id_json, NULL, NULL); + r = json_build(&id_json, JSON_BUILD_OBJECT(JSON_BUILD_PAIR("buildId", JSON_BUILD_HEX(id, id_len)))); if (r < 0) { - log_error_errno(r, "json_parse on %s failed: %m", id_hex_prefixed); + log_error_errno(r, "json_build on build-id failed: %m"); return DWARF_CB_ABORT; } + + build_id = json_variant_by_key(id_json, "buildId"); + assert_se(build_id); + fprintf(c->f, "Found module %s with build-id: %s\n", name, json_variant_string(build_id)); } /* The .note.package metadata is more difficult. From the module, we need to get a reference diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index b49b0ae908..1572483602 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -113,6 +113,11 @@ static void test_path(void) { assert_se(!path_equal_ptr("/a", "/b")); assert_se(!path_equal_ptr("/a", NULL)); assert_se(!path_equal_ptr(NULL, "/a")); + + assert_se(path_equal_filename("/a/c", "/b/c")); + assert_se(path_equal_filename("/a", "/a")); + assert_se(!path_equal_filename("/a/b", "/a/c")); + assert_se(!path_equal_filename("/b", "/c")); } static void test_path_equal_root(void) {