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

[Mark Wielaard <mark@klomp.org>]

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