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

[Mark Wielaard <mark@klomp.org>]

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