[PATCH v2] libebl: recognize FDO Packaging Metadata ELF note
Luca Boccassi
bluca@debian.org
Sat Mar 26 18:19:11 GMT 2022
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://sourceware.org/pipermail/elfutils-devel/attachments/20220326/979c9a07/attachment-0001.sig>
More information about the Elfutils-devel
mailing list