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

[Luca Boccassi <bluca@debian.org>]

[-- 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 --]