[applied] Bug 29345 - abipkgdiff is confused by symlinked binaries in RPMs

Message ID 87bkkemsej.fsf@redhat.com
State New
Headers
Series [applied] Bug 29345 - abipkgdiff is confused by symlinked binaries in RPMs |

Commit Message

Dodji Seketeli March 27, 2023, 9:53 a.m. UTC
  Hello,

Note that this issue can be reproduced by doing:

    $ fedabipkgdiff  --self-compare -a --from fc37 dx

When a binary is a symlink to another one,
create_maps_of_package_content doesn't realize it and considers the
two binaries to be different.

Later, when comes time to self compare both binaries, their abixml
files (which are the same, by virtue of symlinks) might be written at
the same time in different threads, creating a race condition, leading
to corruption of the abixml.

This patch fixes this by teaching create_maps_of_package_content to
resolve symlinks so that it can detect when two files actually point
to the same file.

	* tools/abipkgdiff.cc (create_maps_of_package_content): Resolve
	symlinks when mapping binaries.  Don't map a binary that has
	already been seen.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 tools/abipkgdiff.cc | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)
  

Patch

diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc
index 09adc837..2765284e 100644
--- a/tools/abipkgdiff.cc
+++ b/tools/abipkgdiff.cc
@@ -2460,6 +2460,12 @@  create_maps_of_package_content(package& package, options& opts)
        ++file)
     {
       elf_file_sptr e (new elf_file(*file));
+      string resolved_e_path;
+      // The path 'e->path' might contain symlinks.  Let's resolve
+      // them so we can see if 'e->path' has already been seen before,
+      // for instance.
+      real_path(e->path, resolved_e_path);
+
       if (opts.compare_dso_only)
 	{
 	  if (e->type != abigail::elf::ELF_TYPE_DSO)
@@ -2514,7 +2520,13 @@  create_maps_of_package_content(package& package, options& opts)
 	  // base name.  So let's consider the full path of the binary
 	  // inside the extracted directory.
 	  string key = e->name;
-	  package.convert_path_to_unique_suffix(e->path, key);
+	  package.convert_path_to_unique_suffix(resolved_e_path, key);
+	  if (package.path_elf_file_sptr_map().find(key)
+	      != package.path_elf_file_sptr_map().end())
+	    // 'key' has already been seen before.  So we won't map it
+	    // twice.
+	    continue;
+
 	  package.path_elf_file_sptr_map()[key] = e;
 	  if (opts.verbose)
 	    emit_prefix("abipkgdiff", cerr)
@@ -2544,11 +2556,18 @@  create_maps_of_package_content(package& package, options& opts)
 		}
 	    }
 
-	  if (package.convert_path_to_unique_suffix(e->path, key))
+	  if (package.convert_path_to_unique_suffix(resolved_e_path, key))
 	    {
 	      dir_name(key, key);
 	      key += string("/@soname:") + e->soname;
 	    }
+
+	  if (package.path_elf_file_sptr_map().find(key)
+	      != package.path_elf_file_sptr_map().end())
+	    // 'key' has already been seen before.  So we won't do itl
+	    // twice.
+	    continue;
+
 	  package.path_elf_file_sptr_map()[key] = e;
 	  if (opts.verbose)
 	    emit_prefix("abipkgdiff", cerr)