[PATCH, applied] elf-reader: Don't free CTF resources too early

[PATCH, applied] elf-reader: Don't free CTF resources too early

[Dodji Seketeli]

Hello,

elf::reader::locate_alt_ctf_debug_info frees the memory for the
alternate CTF debug info too early, leading to some segmentation
violation down the road, when the rest of the code tries to access the
CTF section afterwards.  Many thanks to the Valgrind tool and its
hackers for showing me this.

This patch thus keeps the file descriptor and ELF data structure of
the alternate CTF debug info around for the lifetime of the reader.

	* src/abg-elf-reader.cc (reader::priv::{alt_ctf_fd,
	alt_ctf_handle}): Add new data members.
	(reader::priv::clear_alt_ctf_debug_info_data): Define new member
	function.
	(reader::priv::~priv): Call the new
	priv::clear_alt_ctf_debug_info_data
	(reader::priv::initialize): Likewise.  Initialize the new
	alt_ctf_handle and alt_ctf_fd data members.
	(reader::priv::locate_alt_ctf_debug_info): Do not free the fd and
	ELF resources early here.  Store them in the new
	reader::priv::alt_ctf_{fd,handle} instead.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 src/abg-elf-reader.cc | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/src/abg-elf-reader.cc b/src/abg-elf-reader.cc
index 656418e3..16e8b022 100644
--- a/src/abg-elf-reader.cc
+++ b/src/abg-elf-reader.cc
@@ -271,6 +271,8 @@ struct reader::priv
   string				alt_dwarf_path;
   int					alt_dwarf_fd		= 0;
   Elf_Scn*				ctf_section		= nullptr;
+  int					alt_ctf_fd		= 0;
+  Elf*					alt_ctf_handle		= nullptr;
   Elf_Scn*				alt_ctf_section	= nullptr;
 
   priv(reader& reeder, const std::string& elf_path,
@@ -284,6 +286,7 @@ struct reader::priv
   ~priv()
   {
     clear_alt_dwarf_debug_info_data();
+    clear_alt_ctf_debug_info_data();
   }
 
   /// Reset the private data of @elf elf::reader.
@@ -294,6 +297,7 @@ struct reader::priv
   initialize(const vector<char**>& debug_info_roots)
   {
     clear_alt_dwarf_debug_info_data();
+    clear_alt_ctf_debug_info_data();
 
     elf_handle = nullptr;
     symtab_section = nullptr;
@@ -310,6 +314,8 @@ struct reader::priv
     alt_dwarf_fd = 0;
     ctf_section = nullptr;
     alt_ctf_section = nullptr;
+    alt_ctf_handle = nullptr;
+    alt_ctf_fd = 0;
   }
 
   /// Setup the necessary plumbing to open the ELF file and find all
@@ -413,6 +419,22 @@ struct reader::priv
 						 alt_dwarf_fd);
   }
 
+  /// Clear the resources related to the alternate CTF data.
+  void
+  clear_alt_ctf_debug_info_data()
+  {
+    if (alt_ctf_fd)
+      {
+	close(alt_ctf_fd);
+	alt_ctf_fd = 0;
+      }
+    if (alt_ctf_handle)
+      {
+	elf_end(alt_ctf_handle);
+	alt_ctf_handle = nullptr;
+      }
+  }
+
   /// Locate the CTF "alternate" debug information associated with the
   /// current ELF file ( and split out somewhere else).
   ///
@@ -442,23 +464,17 @@ struct reader::priv
 	  if (!tools_utils::find_file_under_dir(*path, name, file_path))
 	    continue;
 
-	  int fd;
-	  if ((fd = open(file_path.c_str(), O_RDONLY)) == -1)
+	  if ((alt_ctf_fd = open(file_path.c_str(), O_RDONLY)) == -1)
 	    continue;
 
-	  Elf *hdl;
-	  if ((hdl = elf_begin(fd, ELF_C_READ, nullptr)) == nullptr)
-	    {
-	      close(fd);
-	      continue;
-	    }
+	  if ((alt_ctf_handle = elf_begin(alt_ctf_fd,
+					  ELF_C_READ,
+					  nullptr)) == nullptr)
+	    continue;
 
 	  // unlikely .ctf was designed to be present in stripped file
 	  alt_ctf_section =
-	    elf_helpers::find_section(hdl, ".ctf", SHT_PROGBITS);
-
-	  elf_end(hdl);
-	  close(fd);
+	    elf_helpers::find_section(alt_ctf_handle, ".ctf", SHT_PROGBITS);
 
 	  if (alt_ctf_section)
 	    break;
-- 
2.31.1


-- 
		Dodji