[PATCH] libebl: recognize FDO Packaging Metadata ELF note

[PATCH] libebl: recognize FDO Packaging Metadata ELF note

[luca.boccassi]

From: Luca Boccassi <bluca@debian.org>

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 <luca.boccassi@microsoft.com>
---
 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..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);
+
       /* 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"
 
-- 
2.30.2

Re: [PATCH] libebl: recognize FDO Packaging Metadata ELF note

[Mark Wielaard]

Hi Luca,

On Fri, Nov 19, 2021 at 12:31:27AM +0000, luca.boccassi--- via Elfutils-devel wrote:
> From: Luca Boccassi <bluca@debian.org>
> 
> 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.

> 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-alpha@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.

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

Re: [PATCH] libebl: recognize FDO Packaging Metadata ELF note

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 3791 bytes --]

On Sun, 2021-11-21 at 17:33 +0100, Mark Wielaard wrote:
> Hi Luca,
> 
> On Fri, Nov 19, 2021 at 12:31:27AM +0000, luca.boccassi--- via
> Elfutils-devel wrote:
> > From: Luca Boccassi <bluca@debian.org>
> > 
> > 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-alpha@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/133330.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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[luca.boccassi]

From: Luca Boccassi <bluca@debian.org>

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 <luca.boccassi@microsoft.com>
---
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"
 
-- 
2.33.0

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 2720 bytes --]

On Sun, 2021-11-21 at 19:43 +0000, luca.boccassi@gmail.com wrote:
> From: Luca Boccassi <bluca@debian.org>
> 
> 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 <luca.boccassi@microsoft.com>
> ---
> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Mark Wielaard]

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 \uXXXX 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 \uXXXX 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 \uXXXX 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

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 2715 bytes --]

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 \uXXXX 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 \uXXXX 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 \uXXXX 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Frank Ch. Eigler]

Hi -

On Tue, Nov 30, 2021 at 12:25:41PM +0100, Mark Wielaard wrote:
> [...]
> 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. [...]
> For Strings it should require that \uXXXX escaping isn't used [...]
> 
> That should get rid of various corner cases that different parsers are
> known to get wrong. 

Are such buggy parsers seen in the wild, now, after all this time with
JSON?  It seems to me it's not really elfutils' or systemd's place to
impose -additional- constraints on customary JSON.


> Especially \uXXXX 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).

Yes, and yet we have had the bidi situation recently where UTF-8 raw
codes could visually confuse a human reader whereas escaped \uXXXX
wouldn't.  If we forbid \uXXXX unilaterally, we literally become
incompatible with JSON (RFC8259 7. String. "Any character may be
escaped."), and for what?

Both these seem to be personal aesthetic decisions that need not be
imposed on a routine application of well-established standards.  We
have many industrial-strength json parser libraries to choose from,
so that part is IMO no longer a timely concern.


- FChE

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Florian Weimer]

* Frank Ch. Eigler via Elfutils-devel:

> Hi -
>
> On Tue, Nov 30, 2021 at 12:25:41PM +0100, Mark Wielaard wrote:
>> [...]
>> 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. [...]
>> For Strings it should require that \uXXXX escaping isn't used [...]
>> 
>> That should get rid of various corner cases that different parsers are
>> known to get wrong. 
>
> Are such buggy parsers seen in the wild, now, after all this time with
> JSON?  It seems to me it's not really elfutils' or systemd's place to
> impose -additional- constraints on customary JSON.

JSON has been targeted at the Windows/Java UTF-16 world, there is always
going to be a mismatch if you try to represent it in UTF-8 or anything
that doesn't have surrogate pairs.

>> Especially \uXXXX 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).
>
> Yes, and yet we have had the bidi situation recently where UTF-8 raw
> codes could visually confuse a human reader whereas escaped \uXXXX
> wouldn't.  If we forbid \uXXXX unilaterally, we literally become
> incompatible with JSON (RFC8259 7. String. "Any character may be
> escaped."), and for what?

RFC 8259 says this:

   However, the ABNF in this specification allows member names and
   string values to contain bit sequences that cannot encode Unicode
   characters; for example, "\uDEAD" (a single unpaired UTF-16
   surrogate).  Instances of this have been observed, for example, when
   a library truncates a UTF-16 string without checking whether the
   truncation split a surrogate pair.  The behavior of software that
   receives JSON texts containing such values is unpredictable; for
   example, implementations might return different values for the length
   of a string value or even suffer fatal runtime exceptions.

A UTF-8 environment has to enforce *some* additional constraints
compared to the official JSON syntax.

Thanks,
Florian

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Mark Wielaard]

Hi,

On Tue, Nov 30, 2021 at 05:49:58PM +0100, Florian Weimer wrote:
> * Frank Ch. Eigler via Elfutils-devel:
> > On Tue, Nov 30, 2021 at 12:25:41PM +0100, Mark Wielaard wrote:
> >> [...]
> >> 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. [...]
> >> For Strings it should require that \uXXXX escaping isn't used [...]
> >> 
> >> That should get rid of various corner cases that different parsers are
> >> known to get wrong. 
> >
> > Are such buggy parsers seen in the wild, now, after all this time with
> > JSON?  It seems to me it's not really elfutils' or systemd's place to
> > impose -additional- constraints on customary JSON.
> 
> JSON has been targeted at the Windows/Java UTF-16 world, there is always
> going to be a mismatch if you try to represent it in UTF-8 or anything
> that doesn't have surrogate pairs.

Right, the \uhex escaping is kind of odd when using UTF-8 because you
suddenly have to use surrogate pairs. It is correct that you may use
them according to the RFC. But the RFC also points out that if you do
there are various odd/bad corner cases to take care of and that it
makes comparing strings really awkward.

> >> Especially \uXXXX 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).
> >
> > Yes, and yet we have had the bidi situation recently where UTF-8 raw
> > codes could visually confuse a human reader whereas escaped \uXXXX
> > wouldn't.  If we forbid \uXXXX unilaterally, we literally become
> > incompatible with JSON (RFC8259 7. String. "Any character may be
> > escaped."), and for what?
> 
> RFC 8259 says this:
> 
>    However, the ABNF in this specification allows member names and
>    string values to contain bit sequences that cannot encode Unicode
>    characters; for example, "\uDEAD" (a single unpaired UTF-16
>    surrogate).  Instances of this have been observed, for example, when
>    a library truncates a UTF-16 string without checking whether the
>    truncation split a surrogate pair.  The behavior of software that
>    receives JSON texts containing such values is unpredictable; for
>    example, implementations might return different values for the length
>    of a string value or even suffer fatal runtime exceptions.
> 
> A UTF-8 environment has to enforce *some* additional constraints
> compared to the official JSON syntax.

Right, all recommendations for encoding the JSON object, string and
numbers actually come from the RFC. I don't think it is odd to
explicitly specify the encoding of the package note as using a zero
terminated UTF-8 string and to specify the allowed JSON string and
number encodings. The contents of the package note is meant to be
human readable, so excluding control characters and excotic escape
sequence in the expected key/value pairs seems natural to me.

Cheers,

Mark

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Frank Ch. Eigler]

Hi -

> JSON has been targeted at the Windows/Java UTF-16 world, there is always
> going to be a mismatch if you try to represent it in UTF-8 or anything
> that doesn't have surrogate pairs.

The JSON RFC8259 8.1 mandates UTF-8 encoding for situations like ours.


> > Yes, and yet we have had the bidi situation recently where UTF-8 raw
> > codes could visually confuse a human reader whereas escaped \uXXXX
> > wouldn't.  If we forbid \uXXXX unilaterally, we literally become
> > incompatible with JSON (RFC8259 7. String. "Any character may be
> > escaped."), and for what?
> 
> RFC 8259 says this:
> 
>    However, the ABNF in this specification allows member names and
>    string values to contain bit sequences that cannot encode Unicode
>    characters; for example, "\uDEAD" (a single unpaired UTF-16
>    surrogate).  Instances of this have been observed, for example, when
>    a library truncates a UTF-16 string without checking whether the
>    truncation split a surrogate pair.  The behavior of software that
>    receives JSON texts containing such values is unpredictable; for
>    example, implementations might return different values for the length
>    of a string value or even suffer fatal runtime exceptions.
> 
> A UTF-8 environment has to enforce *some* additional constraints
> compared to the official JSON syntax.

I'm sorry, I don't see how.  If a JSON string were to include the
suspect "\uDEAD", but from observing our hypothetical "no escapes!"
rule they could reencode it as the UTF-8 octets 0xED 0xBA 0xAD.
ISTM we're no better off.


- FChE

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Florian Weimer]

* Frank Ch. Eigler:

>> > Yes, and yet we have had the bidi situation recently where UTF-8 raw
>> > codes could visually confuse a human reader whereas escaped \uXXXX
>> > wouldn't.  If we forbid \uXXXX unilaterally, we literally become
>> > incompatible with JSON (RFC8259 7. String. "Any character may be
>> > escaped."), and for what?
>> 
>> RFC 8259 says this:
>> 
>>    However, the ABNF in this specification allows member names and
>>    string values to contain bit sequences that cannot encode Unicode
>>    characters; for example, "\uDEAD" (a single unpaired UTF-16
>>    surrogate).  Instances of this have been observed, for example, when
>>    a library truncates a UTF-16 string without checking whether the
>>    truncation split a surrogate pair.  The behavior of software that
>>    receives JSON texts containing such values is unpredictable; for
>>    example, implementations might return different values for the length
>>    of a string value or even suffer fatal runtime exceptions.
>> 
>> A UTF-8 environment has to enforce *some* additional constraints
>> compared to the official JSON syntax.
>
> I'm sorry, I don't see how.  If a JSON string were to include the
> suspect "\uDEAD", but from observing our hypothetical "no escapes!"
> rule they could reencode it as the UTF-8 octets 0xED 0xBA 0xAD.
> ISTM we're no better off.

These octets aren't UTF-8.  UTF-8 never contains surrogate pairs (paired
or unpaired). 8-(

Thanks,
Florian

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Mark Wielaard]

Hi Luca,

On Tue, Nov 30, 2021 at 12:25:41PM +0100, Mark Wielaard wrote:
> 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.

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 for the delay,

Mark

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Mark Wielaard]

> 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

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?

Cheers,

Mark

[PATCH 1/3] libelf: Sync elf.h from glibc.

[Mark Wielaard]

Adds EM_INTELGT, NT_ARM_TAGGED_ADDR_CTRL, NT_ARM_PAC_ENABLED_KEYS,
ELF_NOTE_FDO, NT_FDO_PACKAGING_METADATA and OpenRISC 1000 specific
relocs.

It also adds and renames some GNU_PROPERTY constants. But none of the
constants the elfutils code uses was renamed or given a different
constant value.

dwelf_elf_e_machine_string was updated to handle EM_INTELGT.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 libdwelf/ChangeLog                    |   5 ++
 libdwelf/dwelf_elf_e_machine_string.c |   2 +
 libelf/ChangeLog                      |   4 +
 libelf/elf.h                          | 107 ++++++++++++++++++++------
 4 files changed, 94 insertions(+), 24 deletions(-)

diff --git a/libdwelf/ChangeLog b/libdwelf/ChangeLog
index 5f7fb4ed..c9010af8 100644
--- a/libdwelf/ChangeLog
+++ b/libdwelf/ChangeLog
@@ -1,3 +1,8 @@
+2022-03-24  Mark Wielaard  <mark@klomp.org>
+
+	* dwelf_elf_e_machine_string.c (dwelf_elf_e_machine_string): Add
+	EM_INTELGT Intel Graphics Technology.
+
 2021-02-14  Alexander Miller  <alex.miller@gmx.de>
 
 	* dwelf_elf_begin.c (dwelf_elf_begin): Move NEW_VERSION before
diff --git a/libdwelf/dwelf_elf_e_machine_string.c b/libdwelf/dwelf_elf_e_machine_string.c
index 387648e2..051c70b5 100644
--- a/libdwelf/dwelf_elf_e_machine_string.c
+++ b/libdwelf/dwelf_elf_e_machine_string.c
@@ -360,6 +360,8 @@ dwelf_elf_e_machine_string (int machine)
       return "XMOS xCORE";
     case EM_MCHP_PIC:
       return "Microchip 8-bit PIC";
+    case EM_INTELGT:
+      return "Intel Graphics Technology";
     case EM_KM32:
       return "KM211 KM32";
     case EM_KMX32:
diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 5ea1e41e..7fd6202b 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,7 @@
+2022-03-24  Mark Wielaard  <mark@klomp.org>
+
+	* elf.h: Update from glibc.
+
 2022-03-22  Mark Wielaard  <mark@klomp.org>
 
 	* elf_getdata.c (__libelf_type_aligns): ELF_T_GNUHASH has different
diff --git a/libelf/elf.h b/libelf/elf.h
index 8e3e618f..0735f6b5 100644
--- a/libelf/elf.h
+++ b/libelf/elf.h
@@ -1,5 +1,5 @@
 /* This file defines standard ELF types, structures, and macros.
-   Copyright (C) 1995-2020 Free Software Foundation, Inc.
+   Copyright (C) 1995-2022 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -336,7 +336,8 @@ typedef struct
 #define EM_BA2		202	/* Beyond BA2 */
 #define EM_XCORE	203	/* XMOS xCORE */
 #define EM_MCHP_PIC	204	/* Microchip 8-bit PIC(r) */
-				/* reserved 205-209 */
+#define EM_INTELGT	205	/* Intel Graphics Technology */
+				/* reserved 206-209 */
 #define EM_KM32		210	/* KM211 KM32 */
 #define EM_KMX32	211	/* KM211 KMX32 */
 #define EM_EMX16	212	/* KM211 KMX16 */
@@ -813,6 +814,10 @@ typedef struct
 					   address keys.  */
 #define NT_ARM_PACG_KEYS	0x408	/* ARM pointer authentication
 					   generic key.  */
+#define NT_ARM_TAGGED_ADDR_CTRL	0x409	/* AArch64 tagged address
+					   control.  */
+#define NT_ARM_PAC_ENABLED_KEYS	0x40a	/* AArch64 pointer authentication
+					   enabled keys.  */
 #define NT_VMCOREDD	0x700		/* Vmcore Device Dump Note.  */
 #define NT_MIPS_DSP	0x800		/* MIPS DSP ASE registers.  */
 #define NT_MIPS_FP_MODE	0x801		/* MIPS floating-point mode.  */
@@ -1226,8 +1231,7 @@ typedef struct
 #define AT_L3_CACHESIZE		46
 #define AT_L3_CACHEGEOMETRY	47
 
-#define AT_MINSIGSTKSZ		51 /* Stack needed for signal delivery
-				      (AArch64).  */
+#define AT_MINSIGSTKSZ		51 /* Stack needed for signal delivery  */
 
 /* Note section contents.  Each entry in the note section begins with
    a header of a fixed form.  */
@@ -1254,6 +1258,8 @@ typedef struct
 /* Note entries for GNU systems have this name.  */
 #define ELF_NOTE_GNU		"GNU"
 
+/* Note entries for freedesktop.org have this name.  */
+#define ELF_NOTE_FDO		"FDO"
 
 /* Defined types of notes for Solaris.  */
 
@@ -1297,6 +1303,10 @@ typedef struct
 /* Program property.  */
 #define NT_GNU_PROPERTY_TYPE_0 5
 
+/* Packaging metadata as defined on
+   https://systemd.io/COREDUMP_PACKAGE_METADATA/ */
+#define NT_FDO_PACKAGING_METADATA 0xcafe1a7e
+
 /* Note section name of program property.   */
 #define NOTE_GNU_PROPERTY_SECTION_NAME ".note.gnu.property"
 
@@ -1307,6 +1317,23 @@ typedef struct
 /* No copy relocation on protected data symbol.  */
 #define GNU_PROPERTY_NO_COPY_ON_PROTECTED	2
 
+/* A 4-byte unsigned integer property: A bit is set if it is set in all
+   relocatable inputs.  */
+#define GNU_PROPERTY_UINT32_AND_LO	0xb0000000
+#define GNU_PROPERTY_UINT32_AND_HI	0xb0007fff
+
+/* A 4-byte unsigned integer property: A bit is set if it is set in any
+   relocatable inputs.  */
+#define GNU_PROPERTY_UINT32_OR_LO	0xb0008000
+#define GNU_PROPERTY_UINT32_OR_HI	0xb000ffff
+
+/* The needed properties by the object file.  */
+#define GNU_PROPERTY_1_NEEDED		GNU_PROPERTY_UINT32_OR_LO
+
+/* Set if the object file requires canonical function pointers and
+   cannot be used with copy relocation.  */
+#define GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS (1U << 0)
+
 /* Processor-specific semantics, lo */
 #define GNU_PROPERTY_LOPROC			0xc0000000
 /* Processor-specific semantics, hi */
@@ -1324,31 +1351,26 @@ typedef struct
 
 /* The x86 instruction sets indicated by the corresponding bits are
    used in program.  Their support in the hardware is optional.  */
-#define GNU_PROPERTY_X86_ISA_1_USED		0xc0000000
+#define GNU_PROPERTY_X86_ISA_1_USED		0xc0010002
 /* The x86 instruction sets indicated by the corresponding bits are
    used in program and they must be supported by the hardware.   */
-#define GNU_PROPERTY_X86_ISA_1_NEEDED		0xc0000001
+#define GNU_PROPERTY_X86_ISA_1_NEEDED		0xc0008002
 /* X86 processor-specific features used in program.  */
 #define GNU_PROPERTY_X86_FEATURE_1_AND		0xc0000002
 
-#define GNU_PROPERTY_X86_ISA_1_486		(1U << 0)
-#define GNU_PROPERTY_X86_ISA_1_586		(1U << 1)
-#define GNU_PROPERTY_X86_ISA_1_686		(1U << 2)
-#define GNU_PROPERTY_X86_ISA_1_SSE		(1U << 3)
-#define GNU_PROPERTY_X86_ISA_1_SSE2		(1U << 4)
-#define GNU_PROPERTY_X86_ISA_1_SSE3		(1U << 5)
-#define GNU_PROPERTY_X86_ISA_1_SSSE3		(1U << 6)
-#define GNU_PROPERTY_X86_ISA_1_SSE4_1		(1U << 7)
-#define GNU_PROPERTY_X86_ISA_1_SSE4_2		(1U << 8)
-#define GNU_PROPERTY_X86_ISA_1_AVX		(1U << 9)
-#define GNU_PROPERTY_X86_ISA_1_AVX2		(1U << 10)
-#define GNU_PROPERTY_X86_ISA_1_AVX512F		(1U << 11)
-#define GNU_PROPERTY_X86_ISA_1_AVX512CD		(1U << 12)
-#define GNU_PROPERTY_X86_ISA_1_AVX512ER		(1U << 13)
-#define GNU_PROPERTY_X86_ISA_1_AVX512PF		(1U << 14)
-#define GNU_PROPERTY_X86_ISA_1_AVX512VL		(1U << 15)
-#define GNU_PROPERTY_X86_ISA_1_AVX512DQ		(1U << 16)
-#define GNU_PROPERTY_X86_ISA_1_AVX512BW		(1U << 17)
+/* GNU_PROPERTY_X86_ISA_1_BASELINE: CMOV, CX8 (cmpxchg8b), FPU (fld),
+   MMX, OSFXSR (fxsave), SCE (syscall), SSE and SSE2.  */
+#define GNU_PROPERTY_X86_ISA_1_BASELINE		(1U << 0)
+/* GNU_PROPERTY_X86_ISA_1_V2: GNU_PROPERTY_X86_ISA_1_BASELINE,
+   CMPXCHG16B (cmpxchg16b), LAHF-SAHF (lahf), POPCNT (popcnt), SSE3,
+   SSSE3, SSE4.1 and SSE4.2.  */
+#define GNU_PROPERTY_X86_ISA_1_V2		(1U << 1)
+/* GNU_PROPERTY_X86_ISA_1_V3: GNU_PROPERTY_X86_ISA_1_V2, AVX, AVX2, BMI1,
+   BMI2, F16C, FMA, LZCNT, MOVBE, XSAVE.  */
+#define GNU_PROPERTY_X86_ISA_1_V3		(1U << 2)
+/* GNU_PROPERTY_X86_ISA_1_V4: GNU_PROPERTY_X86_ISA_1_V3, AVX512F,
+   AVX512BW, AVX512CD, AVX512DQ and AVX512VL.  */
+#define GNU_PROPERTY_X86_ISA_1_V4		(1U << 3)
 
 /* This indicates that all executable sections are compatible with
    IBT.  */
@@ -4102,4 +4124,41 @@ enum
 #define R_ARC_TLS_LE_S9		0x4a
 #define R_ARC_TLS_LE_32		0x4b
 
+/* OpenRISC 1000 specific relocs.  */
+#define R_OR1K_NONE		0
+#define R_OR1K_32		1
+#define R_OR1K_16		2
+#define R_OR1K_8		3
+#define R_OR1K_LO_16_IN_INSN	4
+#define R_OR1K_HI_16_IN_INSN	5
+#define R_OR1K_INSN_REL_26	6
+#define R_OR1K_GNU_VTENTRY	7
+#define R_OR1K_GNU_VTINHERIT	8
+#define R_OR1K_32_PCREL		9
+#define R_OR1K_16_PCREL		10
+#define R_OR1K_8_PCREL		11
+#define R_OR1K_GOTPC_HI16	12
+#define R_OR1K_GOTPC_LO16	13
+#define R_OR1K_GOT16		14
+#define R_OR1K_PLT26		15
+#define R_OR1K_GOTOFF_HI16	16
+#define R_OR1K_GOTOFF_LO16	17
+#define R_OR1K_COPY		18
+#define R_OR1K_GLOB_DAT		19
+#define R_OR1K_JMP_SLOT		20
+#define R_OR1K_RELATIVE		21
+#define R_OR1K_TLS_GD_HI16	22
+#define R_OR1K_TLS_GD_LO16	23
+#define R_OR1K_TLS_LDM_HI16	24
+#define R_OR1K_TLS_LDM_LO16	25
+#define R_OR1K_TLS_LDO_HI16	26
+#define R_OR1K_TLS_LDO_LO16	27
+#define R_OR1K_TLS_IE_HI16	28
+#define R_OR1K_TLS_IE_LO16	29
+#define R_OR1K_TLS_LE_HI16	30
+#define R_OR1K_TLS_LE_LO16	31
+#define R_OR1K_TLS_TPOFF	32
+#define R_OR1K_TLS_DTPOFF	33
+#define R_OR1K_TLS_DTPMOD	34
+
 #endif	/* elf.h */
-- 
2.30.2

[PATCH 2/3] libebl: recognize FDO Packaging Metadata ELF note

[Mark Wielaard]

From: Luca Boccassi <bluca@debian.org>

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 <luca.boccassi@microsoft.com>
---
 libebl/ChangeLog            | 5 +++++
 libebl/eblobjnote.c         | 4 ++++
 libebl/eblobjnotetypename.c | 3 +++
 3 files changed, 12 insertions(+)

diff --git a/libebl/ChangeLog b/libebl/ChangeLog
index da690a40..2e31e75d 100644
--- a/libebl/ChangeLog
+++ b/libebl/ChangeLog
@@ -1,3 +1,8 @@
+2021-12-21  Luca Boccassi  <bluca@debian.org>
+
+	* eblobjnote.c (ebl_object_note): Handle NT_FDO_PACKAGING_METADATA.
+	* eblobjnotetypename.c (ebl_object_note_type_name): Likewise.
+
 2021-09-06  Dmitry V. Levin  <ldv@altlinux.org>
 
 	* eblopenbackend.c (openbackend): Remove cast of calloc return value.
diff --git a/libebl/eblobjnote.c b/libebl/eblobjnote.c
index 36efe275..5a7c5c62 100644
--- a/libebl/eblobjnote.c
+++ b/libebl/eblobjnote.c
@@ -288,6 +288,10 @@ 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
+	  && 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..473a1f2f 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 == NT_FDO_PACKAGING_METADATA)
+	return "FDO_PACKAGING_METADATA";
+
       if (strcmp (name, "GNU") != 0)
 	{
 	  /* NT_VERSION is special, all data is in the name.  */
-- 
2.30.2

[PATCH 3/3] elflint: Recognize NT_FDO_PACKAGING_METADATA

[Mark Wielaard]

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 src/ChangeLog | 4 ++++
 src/elflint.c | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/src/ChangeLog b/src/ChangeLog
index 0e705b7d..1e3c31a8 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2022-03-24  Mark Wielaard  <mark@klomp.org>
+
+	* elflint.c (check_note_data): Recognize NT_FDO_PACKAGING_METADATA.
+
 2022-03-11  Mark Wielaard  <mark@klomp.org>
 
 	* addr2line.c (OPT_RELATIVE): New constant.
diff --git a/src/elflint.c b/src/elflint.c
index ef7725ce..d919936f 100644
--- a/src/elflint.c
+++ b/src/elflint.c
@@ -4384,6 +4384,13 @@ section [%2d] '%s': unknown core file note type %" PRIu32
 	    else
 	      goto unknown_note;
 
+	  case NT_FDO_PACKAGING_METADATA:
+	    if (nhdr.n_namesz == sizeof ELF_NOTE_FDO
+		&& strcmp (data->d_buf + name_offset, ELF_NOTE_FDO) == 0)
+	      break;
+	    else
+	      goto unknown_note;
+
 	  case 0:
 	    /* Linux vDSOs use a type 0 note for the kernel version word.  */
 	    if (nhdr.n_namesz == sizeof "Linux"
-- 
2.30.2

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 2034 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Mark Wielaard]

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
  Owner                Data size        Description
readelf: /usr/bin/bash: Warning: note with invalid namesz and/or descsz
found at offset 0x0
readelf: /usr/bin/bash: Warning:  type: 0x7e1afeca, namesize:
0x04000000, descsize: 0x78000000, 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?

> 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.

Cheers,

Mark

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 2819 bytes --]

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
>   Owner                Data size        Description
> readelf: /usr/bin/bash: Warning: note with invalid namesz and/or descsz
> found at offset 0x0
> readelf: /usr/bin/bash: Warning:  type: 0x7e1afeca, namesize:
> 0x04000000, descsize: 0x78000000, 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Mark Wielaard]

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
> >   Owner                Data size        Description
> > readelf: /usr/bin/bash: Warning: note with invalid namesz and/or
> > descsz
> > found at offset 0x0
> > readelf: /usr/bin/bash: Warning:  type: 0x7e1afeca, namesize:
> > 0x04000000, descsize: 0x78000000, 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 <assert.h>
+#include <byteswap.h>
 #include <gelf.h>
 #include <string.h>
 
@@ -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

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.

Thanks,

Mark

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 4027 bytes --]

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
> > >   Owner                Data size        Description
> > > readelf: /usr/bin/bash: Warning: note with invalid namesz and/or
> > > descsz
> > > found at offset 0x0
> > > readelf: /usr/bin/bash: Warning:  type: 0x7e1afeca, namesize:
> > > 0x04000000, descsize: 0x78000000, 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 <assert.h>
> +#include <byteswap.h>
>  #include <gelf.h>
>  #include <string.h>
>  
> 
> 
> 
> @@ -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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Mark Wielaard]

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.

> > 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 linker script please use the proper BYTE,
SHORT, LONG directives to store the ELF note structure values, instead
of a stream of BYTEs, so the linker can take care of the correct value
(endianness) representation for the target arch.

Cheers,

Mark

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 5720 bytes --]

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 linker script please use the proper BYTE,
> SHORT, LONG directives to store the ELF note structure values,
> instead
> of a stream of BYTEs, so the linker can take care of the correct
> value
> (endianness) representation for the target arch.

Generating a binary is actually harder, we tried that first, there is
just too much variation and completely wonky build systems out there.
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?

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 6497 bytes --]

On Sat, 2022-03-26 at 16:57 +0000, 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 +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 linker script please use the proper BYTE,
> > SHORT, LONG directives to store the ELF note structure values,
> > instead
> > of a stream of BYTEs, so the linker can take care of the correct
> > value
> > (endianness) representation for the target arch.
> 
> Generating a binary is actually harder, we tried that first, there is
> just too much variation and completely wonky build systems out there.
> 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!

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Mark Wielaard]

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.

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.

Cheers,

Mark

Re: [PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 2861 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]