diff --git a/_gdb.spec.Patch.include b/_gdb.spec.Patch.include index 0b464fd..38901e1 100644 --- a/_gdb.spec.Patch.include +++ b/_gdb.spec.Patch.include @@ -207,3 +207,7 @@ Patch046: gdb-rhbz2192105-ftbs-dangling-pointer # core file. (RH BZ 2160211) Patch047: gdb-rhbz2160211-excessive-core-file-warnings.patch +# Backport upstream commit f3eee5861743d635 to fix a crash triggered +# when debuginfod makes use of particular openssl settings. +Patch048: gdb-bz2196395-debuginfod-legacy-openssl-crash.patch + diff --git a/_gdb.spec.patch.include b/_gdb.spec.patch.include index 8258dc9..ef55dd0 100644 --- a/_gdb.spec.patch.include +++ b/_gdb.spec.patch.include @@ -45,3 +45,4 @@ %patch -p1 -P045 %patch -p1 -P046 %patch -p1 -P047 +%patch -p1 -P048 diff --git a/_patch_order b/_patch_order index 51c4581..143a63e 100644 --- a/_patch_order +++ b/_patch_order @@ -45,3 +45,4 @@ gdb-rhbz1553104-s390x-arch12-test.patch gdb-binutils29988-read_indexed_address.patch gdb-rhbz2192105-ftbs-dangling-pointer gdb-rhbz2160211-excessive-core-file-warnings.patch +gdb-bz2196395-debuginfod-legacy-openssl-crash.patch diff --git a/gdb-bz2196395-debuginfod-legacy-openssl-crash.patch b/gdb-bz2196395-debuginfod-legacy-openssl-crash.patch new file mode 100644 index 0000000..452fe58 --- /dev/null +++ b/gdb-bz2196395-debuginfod-legacy-openssl-crash.patch @@ -0,0 +1,188 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Andrew Burgess +Date: Tue, 20 Jun 2023 09:46:35 +0100 +Subject: gdb-bz2196395-debuginfod-legacy-openssl-crash.patch + +;; Backport upstream commit f3eee5861743d635 to fix a crash triggered +;; when debuginfod makes use of particular openssl settings. + +gdb/debuginfod: cleanup debuginfod earlier + +A GDB crash was discovered on Fedora GDB that was tracked back to an +issue with the way that debuginfod is cleaned up. + +The bug was reported on Fedora 37, 38, and 39. Here are the steps to +reproduce: + +1. The file /etc/ssl/openssl.cnf contains the following lines: + + [provider_sect] + default = default_sect + ##legacy = legacy_sect + ## + [default_sect] + activate = 1 + + ##[legacy_sect] + ##activate = 1 + + The bug will occur when the '##' characters are removed so that the + lines in question look like this: + + [provider_sect] + default = default_sect + legacy = legacy_sect + + [default_sect] + activate = 1 + + [legacy_sect] + activate = 1 + +2. Clean up any existing debuginfod cache data: + + > rm -rf $HOME/.cache/debuginfod_client + +3. Run GDB: + + > gdb -nx -q -iex 'set trace-commands on' \ + -iex 'set debuginfod enabled on' \ + -iex 'set confirm off' \ + -ex 'start' -ex 'quit' /bin/ls + +set debuginfod enabled on + +set confirm off + Reading symbols from /bin/ls... + Downloading separate debug info for /usr/bin/ls + ... snip ... + Temporary breakpoint 1, main (argc=1, argv=0x7fffffffde38) at ../src/ls.c:1646 + 1646 { + +quit + + Fatal signal: Segmentation fault + ----- Backtrace ----- + ... snip ... + +So GDB ends up crashing during exit. + +What's happening is that when debuginfod is initialised +debuginfod_begin is called (this is in the debuginfod library), this +in turn sets up libcurl, which makes use of openssl. Somewhere during +this setup process an at_exit function is registered to cleanup some +state. + +Back in GDB the debuginfod_client object is managed using this code: + + /* Deleter for a debuginfod_client. */ + + struct debuginfod_client_deleter + { + void operator() (debuginfod_client *c) + { + debuginfod_end (c); + } + }; + + using debuginfod_client_up + = std::unique_ptr; + +And then a global debuginfod_client_up is created to hold a pointer to +the debuginfod_client object. As a global this will be cleaned up +using the standard C++ global object destructor mechanism, which is +run after the at_exit handlers. + +However, it is expected that when debuginfod_end is called the +debuginfod_client object will still be in a usable state, that is, we +don't expect the at_exit handlers to have run and started cleaning up +the library state. + +To fix this issue we need to ensure that debuginfod_end is called +before the at_exit handlers have a chance to run. + +This commit removes the debuginfod_client_up type, and instead has GDB +hold a raw pointer to the debuginfod_client object. We then make use +of GDB's make_final_cleanup to register a function that will call +debuginfod_end. + +As GDB's final cleanups are called before exit is called, this means +that debuginfod_end will be called before the at_exit handlers are +called, and the crash identified above is resolved. + +It's not obvious how this issue can easily be tested for. The bug does +not appear to manifest when using a local debuginfod server, so we'd +need to setup something more involved. For now I'm proposing this +patch without any associated tests. + +diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c +--- a/gdb/debuginfod-support.c ++++ b/gdb/debuginfod-support.c +@@ -96,20 +96,6 @@ struct user_data + ui_out::progress_update progress; + }; + +-/* Deleter for a debuginfod_client. */ +- +-struct debuginfod_client_deleter +-{ +- void operator() (debuginfod_client *c) +- { +- debuginfod_end (c); +- } +-}; +- +-using debuginfod_client_up +- = std::unique_ptr; +- +- + /* Convert SIZE into a unit suitable for use with progress updates. + SIZE should in given in bytes and will be converted into KB, MB, GB + or remain unchanged. UNIT will be set to "B", "KB", "MB" or "GB" +@@ -180,20 +166,45 @@ progressfn (debuginfod_client *c, long cur, long total) + return 0; + } + ++/* Cleanup ARG, which is a debuginfod_client pointer. */ ++ ++static void ++cleanup_debuginfod_client (void *arg) ++{ ++ debuginfod_client *client = static_cast (arg); ++ debuginfod_end (client); ++} ++ ++/* Return a pointer to the single global debuginfod_client, initialising it ++ first if needed. */ ++ + static debuginfod_client * + get_debuginfod_client () + { +- static debuginfod_client_up global_client; ++ static debuginfod_client *global_client = nullptr; + + if (global_client == nullptr) + { +- global_client.reset (debuginfod_begin ()); ++ global_client = debuginfod_begin (); + + if (global_client != nullptr) +- debuginfod_set_progressfn (global_client.get (), progressfn); ++ { ++ /* It is important that we cleanup the debuginfod_client object ++ before calling exit. Some of the libraries used by debuginfod ++ make use of at_exit handlers to perform cleanup. ++ ++ If we wrapped the debuginfod_client in a unique_ptr and relied ++ on its destructor to cleanup then this would be run as part of ++ the global C++ object destructors, which is after the at_exit ++ handlers, which is too late. ++ ++ So instead, we make use of GDB's final cleanup mechanism. */ ++ make_final_cleanup (cleanup_debuginfod_client, global_client); ++ debuginfod_set_progressfn (global_client, progressfn); ++ } + } + +- return global_client.get (); ++ return global_client; + } + + /* Check if debuginfod is enabled. If configured to do so, ask the user diff --git a/gdb.spec b/gdb.spec index 4797705..7653b86 100644 --- a/gdb.spec +++ b/gdb.spec @@ -1252,6 +1252,9 @@ fi %endif %changelog +* Thu Aug 3 2023 Andrew Burgess +- Backport upstream commit f3eee586174, which fixes RHBZ 2196395. + * Wed Jul 19 2023 Fedora Release Engineering - Rebuilt for https://fedoraproject.org/wiki/Fedora_39_Mass_Rebuild