Re: Re: Performance issue with systemd-coredump and container process linking 2000 shared libraries.
> > Which does bring up the question why systemd-coredump isn't running > in > > the same mount space as the crashing program. Then it would simply > find > > the files that the crashing program is using. > > On this point that systemd-coredump might not run in the same mount > namespace, don’t blindly believe me. I think I saw this while > reviewing the > systemd code, but it was the first time I looked at it to investigate > this issue, > so may be wrong. This is correct, in case of containers sd-coredump will run on the host and collect from all the guests, so they are going to be in different namespaces. And even when they are not, the original binary might be long gone by the time it has a chance to run. -- Kind regards, Luca Boccassi signature.asc Description: This is a digitally signed message part
Re: Re: Performance issue with systemd-coredump and container process linking 2000 shared libraries.
On Tue, 20 Jun 2023 at 13:07, Mark Wielaard wrote: > > Hi Luca, > > On Mon, 2023-06-19 at 21:54 +0100, Luca Boccassi wrote: > > > > Which does bring up the question why systemd-coredump isn't running > > > in > > > > the same mount space as the crashing program. Then it would simply > > > find > > > > the files that the crashing program is using. > > > > > > On this point that systemd-coredump might not run in the same mount > > > namespace, don’t blindly believe me. I think I saw this while > > > reviewing the > > > systemd code, but it was the first time I looked at it to investigate > > > this issue, > > > so may be wrong. > > > > This is correct, in case of containers sd-coredump will run on the host > > and collect from all the guests, so they are going to be in different > > namespaces. And even when they are not, the original binary might be > > long gone by the time it has a chance to run. > > Since sd-coredump is run on the host it could transition into the mount > namespace of the process it is capturing the coredump for. This makes > sense if it is then going to do some path based lookups. Alternatively > we could look into making the path based lookups look under > /proc/PID/root/ Since sd-coredump is run as special kernel helper > through /proc/sys/kernel/core_pattern the original executable is still > there (it cannot be reaped till sd-coredump has done its thing). This requires additional privileges that we really don't want to give to sd-coredump, as it fundamentally processes untrusted inputs. Also as mentioned there's no guarantee anyway that the original binary will still be around when the processing happens, it's all asynchronous, so there would still be common corner cases where that doesn't help. There were other suggestions in this thread to fix this issue, to me it seems better to pursue those instead. Kind regards, Luca Boccassi
[PATCH] readelf: add pretty printing for FDO Dlopen Metadata note
From: Luca Boccassi
Note that the webpage in the comment is not published yet,
it will be next week when the next systemd RC is tagged.
The document can be viewed right now on github at:
https://github.com/systemd/systemd/blob/main/docs/ELF_DLOPEN_METADATA.md
But the node ID and the string format are now fixed, even
if the content of the string might change, it will still be
a string.
Signed-off-by: Luca Boccassi
---
libebl/eblobjnote.c | 9 +++--
libebl/eblobjnotetypename.c | 3 +++
libelf/elf.h| 4
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/libebl/eblobjnote.c b/libebl/eblobjnote.c
index 1ba5d8b3..ad3f49de 100644
--- a/libebl/eblobjnote.c
+++ b/libebl/eblobjnote.c
@@ -288,9 +288,14 @@ ebl_object_note (Ebl *ebl, uint32_t namesz, const char
*name, uint32_t type,
if (descsz == 0 && type == NT_VERSION)
return;
- if (strcmp ("FDO", name) == 0 && type == NT_FDO_PACKAGING_METADATA
+ if (strcmp ("FDO", name) == 0
&& descsz > 0 && desc[descsz - 1] == '\0')
- printf("Packaging Metadata: %.*s\n", (int) descsz, desc);
+ {
+ if (type == NT_FDO_PACKAGING_METADATA)
+ printf("Packaging Metadata: %.*s\n", (int) descsz, desc);
+ else if (type == NT_FDO_DLOPEN_METADATA)
+ printf("Dlopen Metadata: %.*s\n", (int) descsz, desc);
+ }
/* Everything else should have the "GNU" owner name. */
if (strcmp ("GNU", name) != 0)
diff --git a/libebl/eblobjnotetypename.c b/libebl/eblobjnotetypename.c
index 473a1f2f..79ff010a 100644
--- a/libebl/eblobjnotetypename.c
+++ b/libebl/eblobjnotetypename.c
@@ -104,6 +104,9 @@ ebl_object_note_type_name (Ebl *ebl, const char *name,
uint32_t type,
if (strcmp (name, "FDO") == 0 && type == NT_FDO_PACKAGING_METADATA)
return "FDO_PACKAGING_METADATA";
+ if (strcmp (name, "FDO") == 0 && type == NT_FDO_DLOPEN_METADATA)
+ return "FDO_DLOPEN_METADATA";
+
if (strcmp (name, "GNU") != 0)
{
/* NT_VERSION is special, all data is in the name. */
diff --git a/libelf/elf.h b/libelf/elf.h
index f2206e5c..bbf4565c 100644
--- a/libelf/elf.h
+++ b/libelf/elf.h
@@ -1336,6 +1336,10 @@ typedef struct
https://systemd.io/COREDUMP_PACKAGE_METADATA/ */
#define NT_FDO_PACKAGING_METADATA 0xcafe1a7e
+/* dlopen metadata as defined on
+ https://systemd.io/ELF_DLOPEN_METADATA/ */
+#define NT_FDO_DLOPEN_METADATA 0x407c0c0a
+
/* Note section name of program property. */
#define NOTE_GNU_PROPERTY_SECTION_NAME ".note.gnu.property"
--
2.39.2
Re: [PATCH] readelf: add pretty printing for FDO Dlopen Metadata note
On Tue, 14 May 2024 at 22:18, Mark Wielaard wrote: > > Hi Luca, > > On Fri, May 10, 2024 at 10:58:02PM +0100, luca.bocca...@gmail.com wrote: > > Note that the webpage in the comment is not published yet, > > it will be next week when the next systemd RC is tagged. > > The document can be viewed right now on github at: > > https://github.com/systemd/systemd/blob/main/docs/ELF_DLOPEN_METADATA.md > > > > But the node ID and the string format are now fixed, even > > if the content of the string might change, it will still be > > a string. > > Not a fan of json, feels very un-ELF. But it is what it is. The patch > looks OK. Could you let us know when the elf.h change is accepted in > glibc, then we'll sync and integrate this. Hi, It is now merged in glibc in git: https://sourceware.org/git/?p=glibc.git;a=commit;h=53f9d74322c831c76bc6cf6ed8941267e8749604
Re: [PATCH] readelf: add pretty printing for FDO Dlopen Metadata note
On Fri, 31 May 2024 at 13:39, Mark Wielaard wrote: > > Hi Luca, > > On Thu, 2024-05-30 at 11:45 +0100, Luca Boccassi wrote: > > On Tue, 14 May 2024 at 22:18, Mark Wielaard wrote: > > > Not a fan of json, feels very un-ELF. But it is what it is. The patch > > > looks OK. Could you let us know when the elf.h change is accepted in > > > glibc, then we'll sync and integrate this. > > > > It is now merged in glibc in git: > > > > https://sourceware.org/git/?p=glibc.git;a=commit;h=53f9d74322c831c76bc6cf6ed8941267e8749604 > > Thanks, I split your original patch in two, and reworded your commit > message now that the constant and URL are published and official. > > [PATCH 1/2] libelf: Sync elf.h from glibc > [PATCH 2/2] readelf: add pretty printing for FDO Dlopen Metadata note Thank you!
Parsing custom note from core file using libdwfl APIs
Hi,
I am trying to use libdwfl parse a core file and extract a custom note
added at link time via a script to the executable that crashes.
I am having issues, as it seems the API to iterate over the PT_NOTE
entries only returns the ones added by the kernel (ie: 6 CORE and 1
LINUX notes).
The note is clearly present in the core, just after the build-id note,
it is visible when opening the core with an hex editor.
The note is also visible in memory after the build-id note when
stepping through the dwfl_segment_report_module() function via gdb, in
the bit where it parses the build-id note. I can see my note just after
it.
I have tried several variations of the following (much shortened for
brevity) snippet, called after dwfl_core_file_report(dwfl, elfl, NULL),
without any luck:
size_t n_program_headers;
elf_getphdrnum(elf, &n_program_headers);
for (size_t i = 0; i < n_program_headers; ++i) {
GElf_Phdr mem;
GElf_Phdr *header = gelf_getphdr(elf, i, &mem);
if (header == NULL || header->p_type != PT_NOTE)
continue;
Elf_Data *data = elf_getdata_rawchunk(elf,
header->p_offset,
header->p_filesz,
header->p_align == 8 ?
ELF_T_NHDR8 : ELF_T_NHDR);
GElf_Nhdr note_header;
size_t offset = 0;
size_t name_offset, desc_offset;
while (offset < data->d_size && (offset = gelf_getnote(data, offset,
¬e_header, &name_offset, &desc_offset)) > 0) {
if (note_header.n_namesz == 0)
continue;
const char *nname = (const char *)data->d_buf + name_offset;
const char *desc = (const char *)data->d_buf + desc_offset;
if (memcmp(nname, "TBD", 3) == 0)
printf("FOUND");
}
}
Any idea what I am doing wrong or what I am missing?
The note is added via the following linker script:
SECTIONS
{
.note.package ALIGN(8): {
BYTE(0x04) BYTE(0x00) BYTE(0x00) BYTE(0x00) /* Length of Owner
including NUL */
BYTE(0x4a) BYTE(0x00) BYTE(0x00) BYTE(0x00) /* Length of Value
including NUL */
BYTE(0x00) BYTE(0x33) BYTE(0xdd) BYTE(0x7a) /* Note ID */
BYTE(0x54) BYTE(0x42) BYTE(0x44) BYTE(0x00) /* Owner: 'TBD\x00' */
BYTE(0x7b) BYTE(0x22) BYTE(0x70) BYTE(0x61) /* Value:
'{"packageType":"deb","package":"fsverity-utils","packageVersion":"1.3-1"}\x00\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(0x64) BYTE(0x65) BYTE(0x62) 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(0x66) BYTE(0x73) BYTE(0x76) BYTE(0x65)
BYTE(0x72) BYTE(0x69) BYTE(0x74) BYTE(0x79)
BYTE(0x2d) BYTE(0x75) BYTE(0x74) BYTE(0x69)
BYTE(0x6c) BYTE(0x73) 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(0x31) BYTE(0x2e)
BYTE(0x33) BYTE(0x2d) BYTE(0x31) BYTE(0x22)
BYTE(0x7d) BYTE(0x00) BYTE(0x00) BYTE(0x00)
}
}
INSERT AFTER .note.gnu.build-id;
--
Kind regards,
Luca Boccassi
signature.asc
Description: This is a digitally signed message part
Storing package metadata in ELF objects
Hello, Cross-posting to the mailing lists of a few relevant projects. After an initial discussion [0], recently we have been working on a new specification [0] to encode rich package-level metadata inside ELF objects, so that it can be included automatically in generated coredump files. The prototype to parse this in systemd-coredump and store the information in systemd-journal is ready for testing and merged upstream. We are now seeking further comments/opinions/suggestions, as we have a few months before the next release and thus there's plenty of time to make incompatible changes to the format and implementation, if required. A proposal to use this by default for all packages built in Fedora 35 has been submitted [1]. The Fedora Wiki and the systemd.io document have more details, but to make a long story short, a new .notes.package section with a JSON payload will be included in ELF objects, encoding various package- build-time information like distro name&version, package name&version, etc. To summarize from the discussion, the main reasons why we believe this is useful are as following: 1) minimal containers: the rpm database is not installed in the containers. The information about build-ids needs to be stored externally, so package name information is not available immediately, but only after offline processing. The new note doesn't depend on the rpm db in any way. 2) handling of a core from a container, where the container and host have different distros 3) self-built and external packages: unless a lot of care is taken to keep access to the debuginfo packages, this information may be lost. The new note is available even if the repository metadata gets lost. Users can easily provide equivalent information in a format that makes sense in their own environment. It should work even when rpms and debs and other formats are mixed, e.g. during container image creation. Other than in Fedora, we are already making the required code changes at Microsoft to use the same format&specification for internally-built binaries, and for tools that parse core files and logs. Tools for RPM and DEB (debhelper) integration are also available [3]. -- Kind regards, Luca Boccassi [0] https://github.com/systemd/systemd/issues/18433 [1] https://systemd.io/COREDUMP_PACKAGE_METADATA/ [2] https://fedoraproject.org/wiki/Changes/Package_information_on_ELF_objects [3] https://github.com/systemd/package-notes signature.asc Description: This is a digitally signed message part
Re: Storing package metadata in ELF objects
On Sat, 2021-04-10 at 13:29 +0100, Luca Boccassi wrote: > Hello, > > Cross-posting to the mailing lists of a few relevant projects. > > After an initial discussion [0], recently we have been working on a new > specification [0] to encode rich package-level metadata inside ELF > objects, so that it can be included automatically in generated coredump > files. The prototype to parse this in systemd-coredump and store the > information in systemd-journal is ready for testing and merged > upstream. We are now seeking further comments/opinions/suggestions, as > we have a few months before the next release and thus there's plenty of > time to make incompatible changes to the format and implementation, if > required. > > A proposal to use this by default for all packages built in Fedora 35 > has been submitted [1]. > > The Fedora Wiki and the systemd.io document have more details, but to > make a long story short, a new .notes.package section with a JSON > payload will be included in ELF objects, encoding various package- > build-time information like distro name&version, package name&version, > etc. > > To summarize from the discussion, the main reasons why we believe this > is useful are as following: > > 1) minimal containers: the rpm database is not installed in the > containers. The information about build-ids needs to be stored > externally, so package name information is not available immediately, > but only after offline processing. The new note doesn't depend on the > rpm db in any way. > > 2) handling of a core from a container, where the container and host > have different distros > > 3) self-built and external packages: unless a lot of care is taken to > keep access to the debuginfo packages, this information may be lost. > The new note is available even if the repository metadata gets lost. > Users can easily provide equivalent information in a format that makes > sense in their own environment. It should work even when rpms and debs > and other formats are mixed, e.g. during container image creation. > > Other than in Fedora, we are already making the required code changes > at Microsoft to use the same format&specification for internally-built > binaries, and for tools that parse core files and logs. > > Tools for RPM and DEB (debhelper) integration are also available [3]. Wrong Fedora list address - off to a great start already :-) (fixed now) -- Kind regards, Luca Boccassi signature.asc Description: This is a digitally signed message part
Re: Storing package metadata in ELF objects
On Fri, 2021-04-30 at 19:57 +0200, Mark Wielaard wrote: > Hi, > > On Sat, 2021-04-10 at 18:44 +, Zbigniew Jędrzejewski-Szmek wrote: > > [I'm forwarding the mail from Luca who is not subscribed to fedora- > > devel] > > On Sat, Apr 10, 2021 at 01:38:31PM +0100, Luca Boccassi wrote: > > Cross-posting to the mailing lists of a few relevant projects. > > Note that in this version of the email the [N] references in your email > don't seem to point anywhere. I found an older variant of the same > email which contained: > > [0] https://github.com/systemd/systemd/issues/18433 > [1] https://systemd.io/COREDUMP_PACKAGE_METADATA/ > [2] https://fedoraproject.org/wiki/Changes/Package_information_on_ELF_objects > [3] https://github.com/systemd/package-notes Sorry about that! Must have messed up the copy&paste. > > After an initial discussion [0], recently we have been working on a new > > specification [0] to encode rich package-level metadata inside ELF > > objects, so that it can be included automatically in generated coredump > > files. The prototype to parse this in systemd-coredump and store the > > information in systemd-journal is ready for testing and merged > > upstream. We are now seeking further comments/opinions/suggestions, as > > we have a few months before the next release and thus there's plenty of > > time to make incompatible changes to the format and implementation, if > > required. > > > > A proposal to use this by default for all packages built in Fedora 35 > > has been submitted [1]. > > > > The Fedora Wiki and the systemd.io document have more details, but to > > make a long story short, a new .notes.package section with a JSON > > payload will be included in ELF objects, encoding various package- > > build-time information like distro name&version, package > > name&version, > > etc. > > Is there a list of default keys (and their canonical spelling, upper- > lower-Camel_Case, etc.)? If there is, could we have a "debuginfod" key > with as value an URL pointing to the debuginfod server URL where the > embedded build-id executable, debuginfo and sources can be found? > https://sourceware.org/elfutils/Debuginfod.html The "Implementation" section of the spec lists the "main" fields: https://systemd.io/COREDUMP_PACKAGE_METADATA/ (source for that is https://github.com/systemd/systemd/blob/main/docs/COREDUMP_PACKAGE_METADATA.md ) Would you like to send a PR to update it and add that field? > > To summarize from the discussion, the main reasons why we believe this > > is useful are as following: > > > > 1) minimal containers: the rpm database is not installed in the > > containers. The information about build-ids needs to be stored > > externally, so package name information is not available immediately, > > but only after offline processing. The new note doesn't depend on the > > rpm db in any way. > > > > 2) handling of a core from a container, where the container and host > > have different distros > > > > 3) self-built and external packages: unless a lot of care is taken to > > keep access to the debuginfo packages, this information may be lost. > > The new note is available even if the repository metadata gets lost. > > Users can easily provide equivalent information in a format that makes > > sense in their own environment. It should work even when rpms and debs > > and other formats are mixed, e.g. during container image creation. > > > > Other than in Fedora, we are already making the required code changes > > at Microsoft to use the same format&specification for internally-built > > binaries, and for tools that parse core files and logs. > > > > Tools for RPM and DEB (debhelper) integration are also available [3]. > > > > > -- > > > Kind regards, > > > Luca Boccassi -- Kind regards, Luca Boccassi signature.asc Description: This is a digitally signed message part
Re: Storing package metadata in ELF objects
On Thu, 2021-05-06 at 03:17 +0200, Mark Wielaard wrote: > Hi Luca, > > On Tue, 2021-05-04 at 14:43 +0100, Luca Boccassi wrote: > > On Fri, 2021-04-30 at 19:57 +0200, Mark Wielaard wrote: > > > Is there a list of default keys (and their canonical spelling, upper- > > > lower-Camel_Case, etc.)? If there is, could we have a "debuginfod" key > > > with as value an URL pointing to the debuginfod server URL where the > > > embedded build-id executable, debuginfo and sources can be found? > > > https://sourceware.org/elfutils/Debuginfod.html > > > > The "Implementation" section of the spec lists the "main" fields: > > > > https://systemd.io/COREDUMP_PACKAGE_METADATA/ > > > > (source for that is > > https://github.com/systemd/systemd/blob/main/docs/COREDUMP_PACKAGE_METADATA.md > > ) > > > > Would you like to send a PR to update it and add that field? > > Sorry, I don't have a github account. But attached is a patch for to > document it and one for the package-notes generator to add an -- > debuginfod argument (maybe the distro should set a default value for > that?) Hopefully those patches could be applied somehow. Hi, Thanks, opened PRs with your commits: https://github.com/systemd/systemd/pull/19523 https://github.com/systemd/package-notes/pull/8 Yes, if the distro has a debuginfod server, it should definitely be included. -- Kind regards, Luca Boccassi signature.asc Description: This is a digitally signed message part
Re: Storing package metadata in ELF objects
On Fri, 2021-05-14 at 12:41 +0200, Guillem Jover wrote: > On Sat, 2021-04-10 at 13:38:31 +0100, Luca Boccassi wrote: > > On Sat, 2021-04-10 at 13:29 +0100, Luca Boccassi wrote: > > > After an initial discussion [0], recently we have been working on a new > > > specification [0] to encode rich package-level metadata inside ELF > > > objects, so that it can be included automatically in generated coredump > > > files. The prototype to parse this in systemd-coredump and store the > > > information in systemd-journal is ready for testing and merged > > > upstream. We are now seeking further comments/opinions/suggestions, as > > > we have a few months before the next release and thus there's plenty of > > > time to make incompatible changes to the format and implementation, if > > > required. > > I've skimmed over the discussion at [0], and while having this data > seems like it might be "nice", I've to agree with the comments there > voicing that there does not really seem to be an actual need and the > overhead and additional work do not seem worth it, TBH, at least > in the Debian context. Hi Guillem, thanks for having a look, much appreciated! Just to clarify, the need is there - this is not an experimental exercise, but it is borne out of an actual need&requirement, and it is undergoing testing right now before deployment in a large scale production infrastructure. Not _everybody_ will need it, and not everywhere - that's absolutely fair, and discussions on whether the ovearhead is worth it for something that is not universally needed, but only in certain use cases, is perfectly reasonable and welcome. I know Zbigniew is going to try and get some raw numbers on the kind of overhead we are talking about, that will hopefully help frame the discussion with more precision. > > > The Fedora Wiki and the systemd.io document have more details, but to > > > make a long story short, a new .notes.package section with a JSON > > > payload will be included in ELF objects, encoding various package- > > > build-time information like distro name&version, package name&version, > > > etc. > > > > > > To summarize from the discussion, the main reasons why we believe this > > > is useful are as following: > > > > > > 1) minimal containers: the rpm database is not installed in the > > > containers. The information about build-ids needs to be stored > > > externally, so package name information is not available immediately, > > > but only after offline processing. The new note doesn't depend on the > > > rpm db in any way. > > In the Debian context, the build-ids data is going to be available > in the affected executables, and in debug symbols packages and the > Packages metaindices listing them, so there's no need for access to > any local dpkg database. Given that someone needing to install debug > packages will need access to those indices (either with outgoing network > access or with a repository mirror), these can be queried at that time. > Not to mention that AFAIR the Debian debug symbol servers make it > possible to query for specific build-ids. This is not strictly related to debug packages, though? In fact, on systems where this could be of most use you explicitly do _not_ install debug packages (or anything at all). Or even if you wanted to, you could not - corefiles are not handled inside the container, but outside. Even if you wanted to and were allowed to (which for many environments it's not the case), you can't install a Debian debug package on a CoreOS host or Mariner host or a Flatcar host. > > > 2) handling of a core from a container, where the container and host > > > have different distros > > How each distribution handles debug packages and debug symbols is > going to be different, so it seems there will be a need for explicit > handling of these, at which point the above mentioned querying can be > implemented as well, w/o the need to encode the packaging data inside > the executable. Again, matching to debug symbols is not the main goal here, build-id works for that. The main goal is to have useful metadata immediately available in all occasions, regardless of where the core was generated on the host, without reaching out to external services, so that it is directly included and collated in the system journal when the core file is handled. With a common metadata definition, there's no need to query or explicitly handle anything - this already works if you use systemd- coredump built from the main branch, and handle a core file from different containers running different distros with binaries having this metadata in the ELF file, and
Re: Storing package metadata in ELF objects
On Wed, 2021-05-19 at 16:58 +0200, Mark Wielaard wrote: > Hi Guillem, > > On Wed, 2021-05-19 at 02:19 +0200, Guillem Jover wrote: > > So this is where I guess I'm missing something. To be able to make > > sense of the coredumps there are two things that might end up being > > relevant, backtraces and source code. systemd-coredump might already > > emit a backtrace, and depending on the information provided it might > > be more or less useful. If one needs the actual debug symbols there's > > already some external querying/fetching required, and if distribution > > specific source code is required because many distributions patch > > upstream source, then even more querying/fetching will be required. > > > > Which is why I'm not seeing why this standalone and isolated metadata > > would be of much help by itself. As in, the way I see it, either the > > information from systemd (w/o the extra metadata) is sufficient to > > track down bugs, or that querying/fetching would be needed anyway, at > > which point the metadata can be inferred too then? > > Because without that metadata you cannot easily figure out where/how to > get the files needed to properly track down the bugs. But using that > metadata you can figure out where the debuginfod server is that can > provide all that information for the binaries/core files (or a distro > specific method if the distributor doesn't have a debuginfod server > yet). Yes, that is a good use case. Another use case is that the person (or bot) who first looks at the journal with the crash notification and the metadata might not necessarily be the one that does the full in-depth debugging. First level support will look at things and go "oh this is from package X at version Y, therefore team Z needs to chime in". Or it can see that it's version A.B.C, which is listed as known buggy, and ignore the issue. Or a myriad of other combinations. Then there are automated systems, parsing logs and creating tickets. Having the structured metadata available in the journal in the crash message means much more complex and useful querying and ticket creation systems can be set up. You _cannot_ just go and query random remote internet servers from these systems, they need to be network-isolated, as logs can have confidential data from customers. The main point is, there are many many things that happen between a crash and the team owning the component looking up debugging symbols and loading core files. This feature is very very useful for a whole lot of those things. > > Oh, I was thinking about those mixed environments, full chroots or > > stripped down containers, from different vendors, but affecting Debian > > installations. What I'm also probably missing here is how does the > > metadata help for a third-party that is not expected to track/keep/upload > > debug symbols nor source packages into some repository, because otherwise > > I'm not seeing how they'd make use of the cores (if they are insufficient > > by themselves) to debug issues? I guess my question back would be what > > would they do with the metadata if they do not have the debug symbols > > nor the sources readily available? Also assuming of course they exercise > > good practices such as never reusing the same package-version-arch tuple > > for different builds, and similar. :) > > Different builds will have different build-ids, so they can be kept > apart. But even if without the distributor having added debuginfod > meta-data and providing a server to fetch the extra symbols, debuginfo, > sources, the extra meta-data is useful to administrators. Just seeing > that crashes are associated with specific distro/version/packages helps > narrow down instabilities. Precisely - again this is a very much real use case. In my case, the engineers doing support and looking at the logs (and only at the logs - no access to the running systems is permitted) _want_ this, because it helps them. Being able to know at a glance to the journal exactly what is borken, with version info, is extremely valuable to them. Yes the version info might not be precise for a minority of use cases that override the binary version with something different than the source version, but that's fine as it's far and few, mostly affects metapackages, and even then it can be documented clearly that the reference is to the source version, not the binary one. -- Kind regards, Luca Boccassi signature.asc Description: This is a digitally signed message part
Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note
On Fri, 2022-03-25 at 00:14 +0100, Mark Wielaard wrote:
> > I haven't forgotten about this. The glibc elf.h change has been
> > integrated now. But when I wanted to resync with the elfutils
> > libelf/elf.h version I noticed something that look like ABI
> > breakage:
> > https://sourceware.org/pipermail/libc-alpha/2021-December/133589.html
> >
> > I am trying to get a response to that before syncing and
> > integrating
> > your patch.
>
> Sorry, I didn't like the answer I got. Basically this is ABI
> breakage,
> it is just that the old constants were never really used, so some
> have
> simply been renamed or given different constant values. Sigh.
>
> That of course is not a good reason to then forget about your
> patch. Apologies.
>
> I took the elf.h update separately. Tweaked your patch a little and
> added a patch of my own to make elflint recognize the new note type.
>
> [PATCH 1/3] libelf: Sync elf.h from glibc.
> [PATCH 2/3] libebl: recognize FDO Packaging Metadata ELF note
> [PATCH 3/3] elflint: Recognize NT_FDO_PACKAGING_METADATA
No problem at all, change looks good, thanks for following up.
> I saw Fedora 36 now has these new package notes. Sadly they omit the
> debugInfoUrl field. Which makes them less useful imho. Do you happen
> to know why that wasn't included?
Not sure, I think the Fedora-side tooling was there already before we
added this to the spec, so it was simply not synced. I'll follow-up and
ask to get it included - might be too late for F36 unfortunately (not
sure new archive-wide rebuilds are going to happen), but if it gets
approved it should be there for F37.
I have included the field in the first PoC that uses the spec in
Debian, for the systemd packages:
$ readelf --notes /usr/lib/systemd/systemd | grep Packaging
Packaging Metadata:
{"type":"deb","os":"debian","name":"systemd","architecture":"amd64","version":"250.4-1","debugInfoUrl":"https://debuginfod.debian.net"}
--
Kind regards,
Luca Boccassi
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note
On Fri, 2022-03-25 at 14:39 +0100, Mark Wielaard wrote:
> Hi Luca,
>
> On Fri, 2022-03-25 at 11:17 +0000, Luca Boccassi wrote:
> > On Fri, 2022-03-25 at 00:14 +0100, Mark Wielaard wrote:
> > > I took the elf.h update separately. Tweaked your patch a little and
> > > added a patch of my own to make elflint recognize the new note
> > > type.
> > >
> > > [PATCH 1/3] libelf: Sync elf.h from glibc.
> > > [PATCH 2/3] libebl: recognize FDO Packaging Metadata ELF note
> > > [PATCH 3/3] elflint: Recognize NT_FDO_PACKAGING_METADATA
> >
> > No problem at all, change looks good, thanks for following up.
>
> Thanks, I pushed these three patches.
> But I noticed an issue on s390x fedora 36.
> This isn't just elfutils though, binutils also has trouble:
>
> Displaying notes found in: .note.package
> OwnerData sizeDescription
> readelf: /usr/bin/bash: Warning: note with invalid namesz and/or descsz
> found at offset 0x0
> readelf: /usr/bin/bash: Warning: type: 0x7e1afeca, namesize:
> 0x0400, descsize: 0x7800, alignment: 4
>
> Note how it seems the sizes are swapped. s390x is a big endian
> platform.
>
> Do you happen to know what/how the notes are created and if that
> process might produce bad little/big encoding issues?
Agh - I knew big endianess was a bad idea! :-)
We have completely overlooked that, the note is created by a linker
script, which is generated at build time by this:
https://github.com/systemd/package-notes/blob/main/generate-package-notes.sh#L254
(well an older version in Fedora, but similar enough)
I'll flag this, thanks for the report
> > I have included the field in the first PoC that uses the spec in
> > Debian, for the systemd packages:
> >
> > $ readelf --notes /usr/lib/systemd/systemd | grep Packaging
> > Packaging Metadata:
> > {"type":"deb","os":"debian","name":"systemd","architecture":"amd64","
> > version":"250.4-1","debugInfoUrl":"https://debuginfod.debian.net"}
>
> Nice, thanks. I'll look into how to pick up the debugInfoUrl and use
> that automagically if possible.
>
> BTW. I notice that Fedora has an osCpe field where Debian has an os
> field. It would imho be good if one or the other got standardized.
Debian has not adopted the CPE spec in its os-release, so there's no
way to 'source' the appropriate value. Also bear in mind the Debian
version is purely a PoC, opt-in and used only in the systemd package,
there's no proposal for distro-wide adoption. I plan to propose that
eventually, but it will not be anytime soon, other distros need to
adopt it first otherwise chances are it will just be rejected outright,
for no particular reason other than "it's a change, we don't like
change".
--
Kind regards,
Luca Boccassi
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note
On Fri, 2022-03-25 at 15:47 +0100, Mark Wielaard wrote:
> Hi Luca,
>
> On Fri, 2022-03-25 at 13:52 +0000, Luca Boccassi wrote:
> > On Fri, 2022-03-25 at 14:39 +0100, Mark Wielaard wrote:
> > > But I noticed an issue on s390x fedora 36.
> > > This isn't just elfutils though, binutils also has trouble:
> > >
> > > Displaying notes found in: .note.package
> > > OwnerData sizeDescription
> > > readelf: /usr/bin/bash: Warning: note with invalid namesz and/or
> > > descsz
> > > found at offset 0x0
> > > readelf: /usr/bin/bash: Warning: type: 0x7e1afeca, namesize:
> > > 0x0400, descsize: 0x7800, alignment: 4
> > >
> > > Note how it seems the sizes are swapped. s390x is a big endian
> > > platform.
> > >
> > > Do you happen to know what/how the notes are created and if that
> > > process might produce bad little/big encoding issues?
> >
> > Agh - I knew big endianess was a bad idea! :-)
> > We have completely overlooked that, the note is created by a linker
> > script, which is generated at build time by this:
> >
> > https://github.com/systemd/package-notes/blob/main/generate-package-notes.sh#L254
> >
> > (well an older version in Fedora, but similar enough)
>
> Yeah, that is definitely wrong. ELF is very careful about endianess. I
> have a patch that detects it and works around it. But it is somewhat
> ugly and has to work very low-level. So I am not sure I really want it.
> Maybe I just apply it as a temporary workaround just for Fedora. But if
> other distros have used such a script to generate package notes they
> are also broken.
>
> diff --git a/libelf/gelf_getnote.c b/libelf/gelf_getnote.c
> index 0f7b9d68..6ef970c5 100644
> --- a/libelf/gelf_getnote.c
> +++ b/libelf/gelf_getnote.c
> @@ -31,6 +31,7 @@
> #endif
>
>
>
>
> #include
> +#include
> #include
> #include
>
>
>
>
> @@ -73,6 +74,22 @@ gelf_getnote (Elf_Data *data, size_t offset, GElf_Nhdr
> *result,
> offset = 0;
> else
> {
> + /* Workaround FDO package notes on big-endian systems,
> + getting namesz and descsz wrong. Detect it by getting
> + a bad namesz, descsz and byte swapped n_type for
> + NT_FDO_PACKAGING_METADATA. */
> + if (unlikely (n->n_type == bswap_32 (NT_FDO_PACKAGING_METADATA)
> + && n->n_namesz > data->d_size
> + && n->n_descsz > data->d_size))
> + {
> + /* n might not be writable, use result and redirect n. */
> + *result = *n;
> + result->n_type = bswap_32 (n->n_type);
> + result->n_namesz = bswap_32 (n->n_namesz);
> + result->n_descsz = bswap_32 (n->n_descsz);
> + n = result;
> + }
> +
> /* This is slightly tricky, offset is guaranteed to be 4
> byte aligned, which is what we need for the name_offset.
> And normally desc_offset is also 4 byte aligned, but not
Thanks, but I don't think it's necessary to apply that just now - this
is a bug and I'll get a fix in the script first in the next couple
days, and then I'll chat with the Fedora dev working on this about what
to do regarding existing binaries.
> Note that Tom (on CC) submitted an IMHO much saner way to generate the
> package notes using simple assembly which would have gotten all this
> correct automagically.
> https://src.fedoraproject.org/fork/tstellar/rpms/package-notes/c/25687ec2d8a4262d5ba5c55d35d68a994b892910
>
> I see you rejected that, but please reconsider. Just hardcoding some
> byte values really is broken.
The reality of having to deal with thirty thousand different build
system, integrated with different tools, and different packaging
systems, with different build scripts, on different distros, means that
ease of integration trumps over everything else. There are packages out
there using build systems that you couldn't even imagine in your worst
nightmares :-)
--
Kind regards,
Luca Boccassi
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note
On Sat, 2022-03-26 at 17:33 +0100, Mark Wielaard wrote:
> Hi Luca,
>
> On Fri, Mar 25, 2022 at 02:55:14PM +0000, Luca Boccassi wrote:
> > On Fri, 2022-03-25 at 15:47 +0100, Mark Wielaard wrote:
> > > > We have completely overlooked that, the note is created by a
> > > > linker
> > > > script, which is generated at build time by this:
> > > >
> > > > https://github.com/systemd/package-notes/blob/main/generate-package-notes.sh#L254
> > > >
> > > > (well an older version in Fedora, but similar enough)
> > >
> > > Yeah, that is definitely wrong. ELF is very careful about
> > > endianess. I
> > > have a patch that detects it and works around it. But it is
> > > somewhat
> > > ugly and has to work very low-level. So I am not sure I really
> > > want it.
> > > Maybe I just apply it as a temporary workaround just for Fedora.
> > > But if
> > > other distros have used such a script to generate package notes
> > > they
> > > are also broken.
> > >
> > > diff --git a/libelf/gelf_getnote.c b/libelf/gelf_getnote.c
> > > [...]
> > > @@ -73,6 +74,22 @@ gelf_getnote (Elf_Data *data, size_t offset,
> > > GElf_Nhdr *result,
> > > offset = 0;
> > > else
> > > {
> > > + /* Workaround FDO package notes on big-endian systems,
> > > + getting namesz and descsz wrong. Detect it by
> > > getting
> > > + a bad namesz, descsz and byte swapped n_type for
> > > + NT_FDO_PACKAGING_METADATA. */
> > > + if (unlikely (n->n_type == bswap_32
> > > (NT_FDO_PACKAGING_METADATA)
> > > + && n->n_namesz > data->d_size
> > > + && n->n_descsz > data->d_size))
> > > + {
> > > + /* n might not be writable, use result and redirect
> > > n. */
> > > + *result = *n;
> > > + result->n_type = bswap_32 (n->n_type);
> > > + result->n_namesz = bswap_32 (n->n_namesz);
> > > + result->n_descsz = bswap_32 (n->n_descsz);
> > > + n = result;
> > > + }
> > > +
> >
> > Thanks, but I don't think it's necessary to apply that just now -
> > this
> > is a bug and I'll get a fix in the script first in the next couple
> > days, and then I'll chat with the Fedora dev working on this about
> > what
> > to do regarding existing binaries.
>
> I did apply it to the Fedora package already:
> https://src.fedoraproject.org/rpms/elfutils/blob/rawhide/f/elfutils-0.186-fdo-swap.patch
> Without it almost all of the selftests fail. And it seems a lot of
> binaries on Fedora have already been compiled with this bogus ELF
> notes in it. The trouble with that is that the notes themselves are
> bad (the sizes are garbage, so anything trying to parse them will
> fail
> and will be unable to parse any notes in the same segement.
Thank you - if we end up rebuilding s390x I'll let you know. The
current segment is clearly bogus, so I'll suggest we do that once the
script is fixed (working on that).
> > > Note that Tom (on CC) submitted an IMHO much saner way to
> > > generate the
> > > package notes using simple assembly which would have gotten all
> > > this
> > > correct automagically.
> > > https://src.fedoraproject.org/fork/tstellar/rpms/package-notes/c/25687ec2d8a4262d5ba5c55d35d68a994b892910
> > >
> > > I see you rejected that, but please reconsider. Just hardcoding
> > > some
> > > byte values really is broken.
> >
> > The reality of having to deal with thirty thousand different build
> > system, integrated with different tools, and different packaging
> > systems, with different build scripts, on different distros, means
> > that
> > ease of integration trumps over everything else. There are packages
> > out
> > there using build systems that you couldn't even imagine in your
> > worst
> > nightmares :-)
>
> I can imagine that, but to be honest I think that is precisely
> because
> you are using a linker script. Best would be to make sure there is
> native support in the linker for this, just like linkers have native
> support for build-ids. Otherwise linking in a simple assembly
> generated note seems a good idea. Linker scripts seem the most
> fragile.
>
> But if you insist using lin
Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note
On Sat, 2022-03-26 at 16:57 +, Luca Boccassi wrote:
> On Sat, 2022-03-26 at 17:33 +0100, Mark Wielaard wrote:
> > Hi Luca,
> >
> > On Fri, Mar 25, 2022 at 02:55:14PM +, Luca Boccassi wrote:
> > > On Fri, 2022-03-25 at 15:47 +0100, Mark Wielaard wrote:
> > > > > We have completely overlooked that, the note is created by a
> > > > > linker
> > > > > script, which is generated at build time by this:
> > > > >
> > > > > https://github.com/systemd/package-notes/blob/main/generate-package-notes.sh#L254
> > > > >
> > > > > (well an older version in Fedora, but similar enough)
> > > >
> > > > Yeah, that is definitely wrong. ELF is very careful about
> > > > endianess. I
> > > > have a patch that detects it and works around it. But it is
> > > > somewhat
> > > > ugly and has to work very low-level. So I am not sure I really
> > > > want it.
> > > > Maybe I just apply it as a temporary workaround just for
> > > > Fedora.
> > > > But if
> > > > other distros have used such a script to generate package notes
> > > > they
> > > > are also broken.
> > > >
> > > > diff --git a/libelf/gelf_getnote.c b/libelf/gelf_getnote.c
> > > > [...]
> > > > @@ -73,6 +74,22 @@ gelf_getnote (Elf_Data *data, size_t offset,
> > > > GElf_Nhdr *result,
> > > > offset = 0;
> > > > else
> > > > {
> > > > + /* Workaround FDO package notes on big-endian
> > > > systems,
> > > > + getting namesz and descsz wrong. Detect it by
> > > > getting
> > > > + a bad namesz, descsz and byte swapped n_type for
> > > > + NT_FDO_PACKAGING_METADATA. */
> > > > + if (unlikely (n->n_type == bswap_32
> > > > (NT_FDO_PACKAGING_METADATA)
> > > > + && n->n_namesz > data->d_size
> > > > + && n->n_descsz > data->d_size))
> > > > + {
> > > > + /* n might not be writable, use result and
> > > > redirect
> > > > n. */
> > > > + *result = *n;
> > > > + result->n_type = bswap_32 (n->n_type);
> > > > + result->n_namesz = bswap_32 (n->n_namesz);
> > > > + result->n_descsz = bswap_32 (n->n_descsz);
> > > > + n = result;
> > > > + }
> > > > +
> > >
> > > Thanks, but I don't think it's necessary to apply that just now -
> > > this
> > > is a bug and I'll get a fix in the script first in the next
> > > couple
> > > days, and then I'll chat with the Fedora dev working on this
> > > about
> > > what
> > > to do regarding existing binaries.
> >
> > I did apply it to the Fedora package already:
> > https://src.fedoraproject.org/rpms/elfutils/blob/rawhide/f/elfutils-0.186-fdo-swap.patch
> > Without it almost all of the selftests fail. And it seems a lot of
> > binaries on Fedora have already been compiled with this bogus ELF
> > notes in it. The trouble with that is that the notes themselves are
> > bad (the sizes are garbage, so anything trying to parse them will
> > fail
> > and will be unable to parse any notes in the same segement.
>
> Thank you - if we end up rebuilding s390x I'll let you know. The
> current segment is clearly bogus, so I'll suggest we do that once the
> script is fixed (working on that).
>
> > > > Note that Tom (on CC) submitted an IMHO much saner way to
> > > > generate the
> > > > package notes using simple assembly which would have gotten all
> > > > this
> > > > correct automagically.
> > > > https://src.fedoraproject.org/fork/tstellar/rpms/package-notes/c/25687ec2d8a4262d5ba5c55d35d68a994b892910
> > > >
> > > > I see you rejected that, but please reconsider. Just hardcoding
> > > > some
> > > > byte values really is broken.
> > >
> > > The reality of having to deal with thirty thousand different
> > > build
> > > system, integrated with different tools, and different packaging
> > > systems, with different build scripts, on different distros,
> > > means
> > > that
> > > ease
Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note
On Mon, 2022-03-28 at 11:57 +0200, Mark Wielaard wrote: > Hi Luca, > > On Sat, 2022-03-26 at 18:19 +0000, Luca Boccassi wrote: > > > Already working on the updated script, the native type is exactly > > > the > > > approach I was taking, this works fine on a Debian machine on s390x > > > (and also on x86_64), eg: > > > > > > -BYTE(0x04) BYTE(0x00) BYTE(0x00) BYTE(0x00) /* Length of > > > Owner including NUL */ > > > -BYTE(0x47) BYTE(0x00) BYTE(0x00) BYTE(0x00) /* Length of > > > Value including NUL */ > > > -BYTE(0x7e) BYTE(0x1a) BYTE(0xfe) BYTE(0xca) /* Note ID */ > > > +LONG(0x04) /* Length of > > > Owner including NUL */ > > > +LONG(0x0047)/* Length of > > > Value including NUL */ > > > +LONG(0xcafe1a7e)/* Note ID */ > > > > > > The rest of the fields are C strings so no change needed, I > > > believe. > > > > > > Does this look right to you as well? > > > > Here's the fix: > > > > https://src.fedoraproject.org/rpms/package-notes/pull-request/3# > > > > Now it's up to the Fedora folks what to do with it. I tested the > > updated script on Debian x86_64 and s390x, and it all works as > > intended. Sorry again for the breakage! > > Yes, that looks correct. Note that the example on > https://systemd.io/COREDUMP_PACKAGE_METADATA/ also uses BYTEs for > everything, instead of LONGs for the namesz, descsz and type words. > > This also seems to make sure everything is aligned (at 4 bytes). An ELF > note is defined as an array of (4 byte) words. Where the first 3 > (n_namesz, n_descsz, n_type) have a special interpretation. Your name > string is also exactly 4 bytes "FDO\0", so you don't need any extra > padding to make the start of the descriptor be aligned. And since you > don't add any other notes to the section you don't need to explicitly > pad the description. The linker should take care of that in case it > merges note sections/segments. Fix for the doc is queued too: https://github.com/systemd/systemd/pull/22879 > Still I would really recommend trying to add native support to linkers > for package notes, just like they support build-ids by default. That > also makes it easier for the linker to simply merge the notes. Trying > to do this with inserting a linker script really feels very fragile. Yes, that would definitely be ideal. But a bit of a chicken-and-egg: we had to get the spec established before there's any realistic change of getting a full new feature merged in BFD, but we can't get it established if we can't use it anywhere. This way we can 'bootstrap' ourselves, and once the idea and usage is more widely accepted, we have a good case to do just that. -- Kind regards, Luca Boccassi signature.asc Description: This is a digitally signed message part
Re: [PATCH] libebl: recognize FDO Packaging Metadata ELF note
On Sun, 2021-11-21 at 17:33 +0100, Mark Wielaard wrote:
> Hi Luca,
>
> On Fri, Nov 19, 2021 at 12:31:27AM +, luca.boccassi--- via
> Elfutils-devel wrote:
> > From: Luca Boccassi
> >
> > As defined on: https://systemd.io/COREDUMP_PACKAGE_METADATA/
> > this note will be used starting from Fedora 36. Allow
> > readelf --notes to pretty print it:
> >
> > Note section [ 3] '.note.package' of 76 bytes at offset 0x2e8:
> > Owner Data size Type
> > FDO 57 FDO_PACKAGING_METADATA
> > Packaging Metadata: {"type":"deb","name":"fsverity-
> > utils","version":"1.3-1"}
>
> Very nice. Thanks,
>
> > diff --git a/libebl/eblobjnote.c b/libebl/eblobjnote.c
> > index 36efe275..1f8bcccf 100644
> > --- a/libebl/eblobjnote.c
> > +++ b/libebl/eblobjnote.c
> > @@ -288,6 +288,9 @@ ebl_object_note (Ebl *ebl, uint32_t namesz, const
> > char *name, uint32_t type,
> > if (descsz == 0 && type == NT_VERSION)
> > return;
> >
> > + if (strcmp ("FDO", name) == 0 && type ==
> > FDO_PACKAGING_METADATA && descsz > 0)
> > + printf(" Packaging Metadata: %.*s\n", (int) descsz, desc);
> > +
>
> We might want to check that the desc is '\0' terminated (although I
> see we also don't do that in other cases, like
> NT_GNU_GOLD_VERSION. But it might be good as a robustness check.
No problem, added in v2.
> > diff --git a/libelf/elf.h b/libelf/elf.h
> > index 8e3e618f..633f9f67 100644
> > --- a/libelf/elf.h
> > +++ b/libelf/elf.h
> > @@ -1297,6 +1297,9 @@ typedef struct
> > /* Program property. */
> > #define NT_GNU_PROPERTY_TYPE_0 5
> >
> > +/* Packaging metadata as defined on
> > https://systemd.io/COREDUMP_PACKAGE_METADATA/ */
> > +#define FDO_PACKAGING_METADATA 0xcafe1a7e
> > +
> > /* Note section name of program property. */
> > #define NOTE_GNU_PROPERTY_SECTION_NAME ".note.gnu.property"
>
> Would you mind posting the elf.h patch to glibc-al...@sourceware.org.
> We normally sync elf.h with the glibc one. It will also make sure
> other users of elf.h also get the new constants.
Sure, done:
https://sourceware.org/pipermail/libc-alpha/2021-November/10.html
> As a followup I wouldn't mind a minimal testcase.
> Especially if it contains a debuginfod url.
>
> We would have to think how to integrate that with libdw
> dwfl_build_id_find_elf and dwfl_standard_find_debuginfo which use
> debuginfod_find from the debuginfod-client library.
>
> Since the payload of the FDO_PACKAGING_METADATA note are not simply
> key/values, but encoded in json, so we will need to add or depend on a
> json parser. Any recommendations? It seems a simple enough format to
> just write our own (especially if we can simply skip everything except
> top-level key/value strings to find the debuginfod-url).
>
> Thanks,
>
> Mark
Popular C parsers that I know of are json-c and jannson:
https://github.com/json-c/json-c/wiki
https://digip.org/jansson/
json-c seems to be available in slightly more places:
https://repology.org/project/json-c/versions
https://repology.org/project/jansson/versions
Rolling your own full parser can always end up being tricky and a lot
of work, especially for limited usage with no particular requirements.
You need to ensure you've got good fuzzing, etc. If using one of the
above is optional and tied to the debuginfod feature being enabled,
there shouldn't be issues with bootstrapping.
A simple search for the "debugInfoUrl" string, using whatever string is
quoted next as the url would be much simpler of course, if that's all
you need. Up to you of course.
--
Kind regards,
Luca Boccassi
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note
On Sun, 2021-11-21 at 19:43 +, luca.bocca...@gmail.com wrote:
> From: Luca Boccassi
>
> As defined on: https://systemd.io/COREDUMP_PACKAGE_METADATA/
> this note will be used starting from Fedora 36. Allow
> readelf --notes to pretty print it:
>
> Note section [ 3] '.note.package' of 76 bytes at offset 0x2e8:
> Owner Data size Type
> FDO 57 FDO_PACKAGING_METADATA
> Packaging Metadata:
> {"type":"deb","name":"fsverity-utils","version":"1.3-1"}
>
> Signed-off-by: Luca Boccassi
> ---
> v2: check that note payload is NULL terminated
>
> libebl/eblobjnote.c | 3 +++
> libebl/eblobjnotetypename.c | 3 +++
> libelf/elf.h| 3 +++
> 3 files changed, 9 insertions(+)
>
> diff --git a/libebl/eblobjnote.c b/libebl/eblobjnote.c
> index 36efe275..e3ad1409 100644
> --- a/libebl/eblobjnote.c
> +++ b/libebl/eblobjnote.c
> @@ -288,6 +288,9 @@ ebl_object_note (Ebl *ebl, uint32_t namesz, const char
> *name, uint32_t type,
> if (descsz == 0 && type == NT_VERSION)
> return;
>
>
>
>
> + if (strcmp ("FDO", name) == 0 && type == FDO_PACKAGING_METADATA &&
> descsz > 0 && desc[descsz - 1] == '\0')
> + printf("Packaging Metadata: %.*s\n", (int) descsz, desc);
> +
> /* Everything else should have the "GNU" owner name. */
> if (strcmp ("GNU", name) != 0)
> return;
> diff --git a/libebl/eblobjnotetypename.c b/libebl/eblobjnotetypename.c
> index 4662906d..863f20e4 100644
> --- a/libebl/eblobjnotetypename.c
> +++ b/libebl/eblobjnotetypename.c
> @@ -101,6 +101,9 @@ ebl_object_note_type_name (Ebl *ebl, const char *name,
> uint32_t type,
> return buf;
> }
>
>
>
>
> + if (strcmp (name, "FDO") == 0 && type == FDO_PACKAGING_METADATA)
> + return "FDO_PACKAGING_METADATA";
> +
> if (strcmp (name, "GNU") != 0)
> {
> /* NT_VERSION is special, all data is in the name. */
> diff --git a/libelf/elf.h b/libelf/elf.h
> index 8e3e618f..633f9f67 100644
> --- a/libelf/elf.h
> +++ b/libelf/elf.h
> @@ -1297,6 +1297,9 @@ typedef struct
> /* Program property. */
> #define NT_GNU_PROPERTY_TYPE_0 5
>
>
>
>
> +/* Packaging metadata as defined on
> https://systemd.io/COREDUMP_PACKAGE_METADATA/ */
> +#define FDO_PACKAGING_METADATA 0xcafe1a7e
> +
> /* Note section name of program property. */
> #define NOTE_GNU_PROPERTY_SECTION_NAME ".note.gnu.property"
Hi Mark,
I could move this definition to an internal header if the change to
elf.h blocks this patch, if you prefer? Let me know.
--
Kind regards,
Luca Boccassi
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note
On Tue, 2021-11-30 at 12:25 +0100, Mark Wielaard wrote: > Hi Luca, > > On Thu, 2021-11-25 at 17:02 +0000, Luca Boccassi via Elfutils-devel > wrote: > > +/* Packaging metadata as defined on > > > https://systemd.io/COREDUMP_PACKAGE_METADATA/ */ > > > +#define FDO_PACKAGING_METADATA 0xcafe1a7e > > > + > > > /* Note section name of program property. */ > > > #define NOTE_GNU_PROPERTY_SECTION_NAME ".note.gnu.property" > > > > I could move this definition to an internal header if the change to > > elf.h blocks this patch, if you prefer? Let me know. > > It looks like it will be integrated into glibc elf.h later this week. > I'll resync elf.h then and apply the other half of your patch. > > While looking at how to implement the json parsing of the note data I > noticed a couple of things that could be clarified in the spec to make > this more clear and less ambiguous. > > The spec says "a key-value JSON format", "JSON payload" and "a JSON > string with the structure described below". Which isn't very exact, or > seems to describe multiple different JSON concepts which aren't exactly > the same thing. A JSON string is something different from a JSON object > (which is the only JSON value that has a key-value format). And it > doesn't really make clear what the encoding of the JSON itself is (it > cannot be a JSON string, because that itself is expressed in an > specific character encoding itself). > > What I think the spec should say is something like: > "The note data is a single JSON object encoded as a zero terminated > UTF-8 string." > > The spec does explain the requirements for JSON numbers, but doesn't > mention any for JSON strings or JSON objects. It would be good to also > explain how to make the strings and objects unambiguous. > > For Objects it should require that all names are unique. See section 4 > in rfc8259. > > For Strings it should require that \u escaping isn't used and that > only control characters that have an explicit escape sequence are used > (\b, \n, \f, \r, \t) [if you allow them in the first place, they are > probably confusing and it may be good to simply say that Strings should > not contain any control characters or use \u escaping]. See section > 7 and section 8 in rfc8259. > > That should get rid of various corner cases that different parsers are > known to get wrong. Especially \u escaping is really confusing when > using the UTF-8 encoding (and it is totally necessary since UTF-8 can > express any valid UTF character already). > > Cheers, > > Mark Thanks, see: https://github.com/systemd/systemd/pull/21578 -- Kind regards, Luca Boccassi signature.asc Description: This is a digitally signed message part
