Re: [obv] [patch] debuginfod-client memory hygiene
Hi Frank, On Thu, Feb 20, 2025 at 02:18:04PM -0500, Frank Ch. Eigler wrote: > > [...] > > This does sounds like a bug in glibc sscanf. I cannot find a > > description of what exactly happens with 'm' modifier allocated > > buffers on error. So I can imagine a double free if sscanf frees the > > buffer on error. But returning a bogus pointer? That seems a bug. If > > we aren't guaranteed a valid pointer (or NULL) then this could easily > > lead to memory leaks. > > Spent way too long trying to reproduce this. It was a PEBCAK on my > part, comparing a version of elfutils with the distro, and a > locally-built one that included this commit. I think I must have > mixed up some LD_LIBRARY_PATH and run a franken-binary, then misplaced > the blame for the crash. I'll revert my unnecessary fix. At least you confirmed this can happen in practice. I must admit I had assumed this was just a theoretical fix, but that the compiler wouldn't actually produce code where the variable wasn't initialized. > (It might be nice if the fedora build got this fix sometime.) > > > commit 1be0787d6654ed71bf659e8bfd34895fea7589eb > Author: Aaron Merey > Date: Fri Jan 24 19:43:19 2025 -0500 > > debuginfod-client.c: Avoid freeing uninitialized value Added it to the fedora elfutils package. In general it might be good to make a new elfutils release. We have been accumulating various fixes since 0.192. Fedora is carrying 8 backports (plus an odd s390x endian fixup, that is probably not necessary anymore). https://src.fedoraproject.org/rpms/elfutils/tree/rawhide Cheers, Mark
☺ Buildbot (Sourceware): elfutils - build successful (main)
A restored build has been detected on builder elfutils-fedora-s390x while building elfutils. Full details are available at: https://builder.sourceware.org/buildbot/#/builders/43/builds/396 Build state: build successful Revision: e543c7f5c2b28ac2bce1e9f09fad30caebb579d5 Worker: fedora-s390x Build Reason: (unknown) Blamelist: Frank Ch. Eigler Steps: - 0: worker_preparation ( success ) - 1: set package name ( success ) - 2: git checkout ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/2/logs/stdio - 3: autoreconf ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/3/logs/stdio - 4: configure ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/4/logs/stdio - config.log: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/4/logs/config_log - 5: get version ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/5/logs/stdio - property changes: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/5/logs/property_changes - 6: make ( warnings ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/6/logs/stdio - warnings (3): https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/6/logs/warnings__3_ - 7: make check ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/7/logs/stdio - test-suite.log: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/7/logs/test-suite_log - 8: prep ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/8/logs/stdio - 9: build bunsen.cpio.gz ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/9/logs/stdio - 10: fetch bunsen.cpio.gz ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/10/logs/stdio - 11: unpack bunsen.cpio.gz ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/11/logs/stdio - 12: pass .bunsen.source.* ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/12/logs/stdio - 13: upload to bunsen ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/13/logs/stdio - 14: clean up ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/14/logs/stdio - 15: make distclean ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/43/builds/396/steps/15/logs/stdio
Re: ☠ Buildbot (Sourceware): elfutils-snapshots-coverage - failed test (failure) (main)
Hi, On Thu, Feb 20, 2025 at 08:18:15PM +, buil...@sourceware.org wrote: > A new failure has been detected on builder elfutils-snapshots-coverage while > building elfutils. > > Full details are available at: > https://builder.sourceware.org/buildbot/#/builders/250/builds/210 > > Build state: failed test (failure) > Revision: e543c7f5c2b28ac2bce1e9f09fad30caebb579d5 > Worker: snapshots > Build Reason: (unknown) > Blamelist: Frank Ch. Eigler This was a timeout running the tests. I don't immediately know why. Triggering a rebuild made the coverage report snapshot work again: https://snapshots.sourceware.org/elfutils/coverage/latest/ Cheers, Mark
[Bug backends/32684] aarch64 linux 4 build failure: struct user_pac_mask not defined
https://sourceware.org/bugzilla/show_bug.cgi?id=32684
Mark Wielaard changed:
What|Removed |Added
Status|UNCONFIRMED |RESOLVED
Resolution|--- |FIXED
--- Comment #3 from Mark Wielaard ---
commit 52a747a316042e70a22acb489df9e51bfc6bf2d5
Author: Markus Mayer
Date: Fri Feb 21 11:19:34 2025 -0800
aarch64: define struct user_pac_mask if needed
On Aarch64, Linux is using Pointer Authentication Code (PAC) for pointer
authentication.[1] The struct "user_pac_mask" has been part of the Linux
kernel since version 5.0 as part of this feature. However, older kernels
do not define it.
Therefore, we want to check if the definition is present in the kernel
headers and provide one if it isn't. This ensures two things:
* elfutils will continue to compile against kernel headers from 4.x
* binaries built against older kernel headers will still be fully
functional if used on a newer system
For reference, the build error that is being avoided looks like this:
[...]
CC aarch64_initreg.o
aarch64_initreg.c: In function 'aarch64_set_initial_registers_tid':
aarch64_initreg.c:61:24: error: storage size of 'pac_mask' isn't known
struct user_pac_mask pac_mask;
^~~~
aarch64_initreg.c:61:24: warning: unused variable 'pac_mask'
[-Wunused-varia
ble]
make[4]: *** [Makefile:831: aarch64_initreg.o] Error 1
make[3]: *** [Makefile:547: all-recursive] Error 1
make[2]: *** [Makefile:463: all] Error 2
[1] https://docs.kernel.org/arch/arm64/pointer-authentication.html
https://sourceware.org/bugzilla/show_bug.cgi?id=32684
Fixes: 64e3b451ad2c ("aarch64: extend dwfl_thread_state_registers to handle
PAC")
Signed-off-by: Markus Mayer
--
You are receiving this mail because:
You are on the CC list for the bug.
Re: [PATCH] aarch64: define struct user_pac_mask if needed
Hi Markus, On Fri, Feb 21, 2025 at 11:19:34AM -0800, Markus Mayer wrote: > On Aarch64, Linux is using Pointer Authentication Code (PAC) for pointer > authentication.[1] The struct "user_pac_mask" has been part of the Linux > kernel since version 5.0 as part of this feature. However, older kernels > do not define it. > > Therefore, we want to check if the definition is present in the kernel > headers and provide one if it isn't. This ensures two things: > > * elfutils will continue to compile against kernel headers from 4.x > * binaries built against older kernel headers will still be fully > functional if used on a newer system This looks very nice. I added a reference to https://sourceware.org/bugzilla/show_bug.cgi?id=32684 to the commit message and pushed your commit. Thanks, Mark
[PATCH] libelf: Rewrite elf_scnshndx, extended index table handling
elf_scnshndx is a elfutils extension to libelf that given a SHT_SYMTAB section returns the index to the corresponding SHT_SYMTAB_SHNDX section, if it exists. This is needed when there are more than 64K sections and there are symbols that have to refer to a section with an index larger than 64K, because the Elf Sym st_shndx field is only 16 bits. This was implemented by adding an shndx_index field to the Elf_Scn struct which is updated when reading the section headers. This takes up space in every section and is hard to proof correct. In the case of using ELF_C_READ_MMAP the shndx_index field was only updated when the shdrs needed to be converted from file to memory order. And the two places were this function was used in readelf.c and elf-print-reloc-syms.c the wrong section was used to lookup the extended index table. There were also no tests for this functionality. Replace the elf_scnshndx implementation with a simpler lookup over all sections. This sounds inefficient, but in practice the SHT_SYMTAB_SHNDX section is the next section after the SHT_SYMTAB section. elf_scnshndx only needs to be called when there are more than SHN_LORESERVE (0xff00) sections. And normally a user would just lookup the SHT_SYMTAB and SHT_SYMTAB_SHNDX sections at the same time (which is what readelf does when showing the symbol table, as does nm, objcopy and libdwfl). Add a testfile manyfuncs.c that when compiled contains 64K symbols and sections. Make sure to use -fasynchronous-unwind-tables so there is at least one relocatable section that uses all function symbols (e.g. on arm32 where there is no .eh_frame by default). This can then be used to verify the readelf --relocs support. Add another test, test-manyfuncs that explicitly goes through the symbol table and associated extended index table and verify each function symbol matches the section name. There are For riscv there are local, notype, symbols at the start of each executable section which relocations refer to instead of the section symbol. Since all these local symbols are called ".L0" this isn't very useful, so print the section name instead. For powerpc ELFv1 all function symbols go through the .opd section. Allow this in the new test-manyfuncs test. * libelf/elf32_getshdr.c (load_shdr_wrlock): Remove handling of shndx_index. * libelf/elf_begin.c (file_read_elf): Likewise. * libelf/elf_scnshndx.c (elf_scnshndx): Rewritten. * libelf/libelf.h (elf_scnshndx): Added full documentation. * libelf/libelfP.h (struct Elf_Scn): Remove shndx_index field. (__elf_scnshndx_internal): Removed. * src/readelf.c (handle_relocs_rel): Use symscn in call to elf_scnshndx. Print section name for local start section label. (handle_relocs_rela): Likewise. * tests/Makefile.am (check_PROGRAMS): Add test-manyfuncs. (manyfuncs.o): New target. (check-local): New target, depends on manyfuncs.o. (TESTS): Add run-readelf-r-manyfuncs.sh and run-test-manyfuncs.sh. (EXTRA_DIST): Add run-readelf-r-manyfuncs.sh, run-test-manyfuncs.sh and manyfuncs.c. (test_manyfuncs_LDADD): New variable. (EXTRA_test_manyfuncs_DEPENDENCIES): New variable. (CLEANFILES): Add manyfuncs.o. * tests/elf-print-reloc-syms.c (print_reloc_symnames): Use symscn in call to elf_scnshndx. * tests/manyfuncs.c: New test file to generate 64K symbols and sections. * tests/run-readelf-r-manyfuncs.sh: New test wrapper. * tests/run-test-manyfuncs.sh: Likewise. * tests/test-manyfuncs.c: New test. Signed-off-by: Mark Wielaard --- libelf/elf32_getshdr.c | 14 -- libelf/elf_begin.c | 26 --- libelf/elf_scnshndx.c| 52 - libelf/libelf.h | 9 +- libelf/libelfP.h | 4 - src/readelf.c| 32 ++- tests/Makefile.am| 26 ++- tests/elf-print-reloc-syms.c | 4 +- tests/manyfuncs.c| 53 + tests/run-readelf-r-manyfuncs.sh | 40 tests/run-test-manyfuncs.sh | 23 +++ tests/test-manyfuncs.c | 343 +++ 12 files changed, 560 insertions(+), 66 deletions(-) create mode 100644 tests/manyfuncs.c create mode 100755 tests/run-readelf-r-manyfuncs.sh create mode 100755 tests/run-test-manyfuncs.sh create mode 100644 tests/test-manyfuncs.c diff --git a/libelf/elf32_getshdr.c b/libelf/elf32_getshdr.c index 19b690a8e87b..e4bebe18f225 100644 --- a/libelf/elf32_getshdr.c +++ b/libelf/elf32_getshdr.c @@ -146,20 +146,6 @@ load_shdr_wrlock (Elf_Scn *scn) CONVERT_TO (shdr[cnt].sh_addralign, notcvt[cnt].sh_addralign); CONVERT_TO (shdr[cnt].sh_entsize, notcvt[cnt].sh_entsize); - - /* If this is a section with an extended index add a -reference in the section which uses the extended -
