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
signature.asc
Description: This is a digitally signed message part
