[archive, plugin] avoid recursing on symbolic link loops

It's possible that symlink loops exist in the host file system,
either 'simple' ('a'->'a'), or indirect ('a'->'b'->'a'). We need
to avoid recursing on these loops, to avoid exceeding the maximum
link or recursion depths, but we should still represent these
inodes as accurately as possible in the resulting archive.

Detect loops in both the Plugin link handling code and in the new
Archive link follow-up code by creating the first requested level
of loop, and then skipping the recursive follow-up. This means
that the looping links are still created in the archive so long
as they are referenced in a copy spec but that we do not attempt
to indefinitely recurse while collecting them.

Resolves: #1430

Signed-off-by: Bryn M. Reeves <bmr@redhat.com>

2 files changed, 42 insertions, 5 deletions

diff --git a/sos/archive.py b/sos/archive.py
index 483d66f4..e5819432 100644
--- a/sos/archive.py
+++ b/sos/archive.py
@@ -424,6 +424,29 @@ class FileCacheArchive(Archive):
         host_path_name = os.path.realpath(os.path.join(source_dir, source))
         dest_path_name = self.dest_path(host_path_name)
 
+        def is_loop(link_name, source):
+            """Return ``True`` if the symbolic link ``link_name`` is part
+                of a file system loop, or ``False`` otherwise.
+            """
+            link_dir = os.path.dirname(link_name)
+            if not os.path.isabs(source):
+                source = os.path.realpath(os.path.join(link_dir, source))
+            link_name = os.path.realpath(link_name)
+
+            # Simple a -> a loop
+            if link_name == source:
+                return True
+
+            # Find indirect loops (a->b-a) by stat()ing the first step
+            # in the symlink chain
+            try:
+                os.stat(link_name)
+            except OSError as e:
+                if e.errno == 40:
+                    return True
+                raise
+            return False
+
         if not os.path.exists(dest_path_name):
             if os.path.islink(host_path_name):
                 # Normalised path for the new link_name
@@ -433,6 +456,10 @@ class FileCacheArchive(Archive):
                 # Relative source path of the new link
                 source = os.path.join(dest_dir, os.readlink(host_path_name))
                 source = os.path.relpath(source, dest_dir)
+                if is_loop(link_name, source):
+                    self.log_debug("Link '%s' - '%s' loops: skipping..." %
+                                   (link_name, source))
+                    return
                 self.log_debug("Adding link %s -> %s for link follow up" %
                                (link_name, source))
                 self.add_link(source, link_name)
diff --git a/sos/plugins/__init__.py b/sos/plugins/__init__.py
index 7d011a02..7d2a8b2d 100644
--- a/sos/plugins/__init__.py
+++ b/sos/plugins/__init__.py
@@ -376,6 +376,21 @@ class Plugin(object):
             self._log_debug("link '%s' is a directory, skipping..." % linkdest)
             return
 
+        self.copied_files.append({'srcpath': srcpath,
+                                  'dstpath': dstpath,
+                                  'symlink': "yes",
+                                  'pointsto': linkdest})
+
+        # Check for indirect symlink loops by stat()ing the next step
+        # in the link chain.
+        try:
+            os.stat(absdest)
+        except OSError as e:
+            if e.errno == 40:
+                self._log_debug("link '%s' is part of a file system "
+                                "loop, skipping target..." % dstpath)
+                return
+
         # copy the symlink target translating relative targets
         # to absolute paths to pass to _do_copy_path.
         self._log_debug("normalized link target '%s' as '%s'"
@@ -388,11 +403,6 @@ class Plugin(object):
             self._log_debug("link '%s' points to itself, skipping target..."
                             % linkdest)
 
-        self.copied_files.append({'srcpath': srcpath,
-                                  'dstpath': dstpath,
-                                  'symlink': "yes",
-                                  'pointsto': linkdest})
-
     def _copy_dir(self, srcpath):
         try:
             for afile in os.listdir(srcpath):