From 9a0ab16793a8388b2c3d3909fd3a087c5b6296d4 Mon Sep 17 00:00:00 2001

From: Pavel Moravec <pmoravec@redhat.com>

Date: Fri, 1 Nov 2019 12:13:23 -0400

Subject: [PATCH 01/10] [Plugin] remove invalid {strip/join}_sysroot()

Do not strip the sysroot path prefix when calling _do_copy_path()

for a symlink target and do not add the sysroot prefix when

testing for a forbidden path.

Related: #1842

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>

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

---

 sos/plugins/__init__.py | 4 +---

 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sos/plugins/__init__.py b/sos/plugins/__init__.py

index e75ec82e..4f1b73ce 100644

--- a/sos/plugins/__init__.py

+++ b/sos/plugins/__init__.py

@@ -731,7 +731,7 @@ class Plugin(object):

         # skip recursive copying of symlink pointing to itself.

         if (absdest != srcpath):

-            self._do_copy_path(self.strip_sysroot(absdest))

+            self._do_copy_path(absdest)

         else:

             self._log_debug("link '%s' points to itself, skipping target..."

                             % linkdest)

@@ -758,8 +758,6 @@ class Plugin(object):

         return None

     def _is_forbidden_path(self, path):

-        if self.use_sysroot():

-            path = self.join_sysroot(path)

         return _path_in_path_list(path, self.forbidden_paths)

     def _copy_node(self, path, st):

-- 

2.21.0

From aeeebf126fc9fdb0fd8c3b01418bef742bce78c3 Mon Sep 17 00:00:00 2001

From: "Bryn M. Reeves" <bmr@redhat.com>

Date: Fri, 1 Nov 2019 12:22:51 -0400

Subject: [PATCH 02/10] [Plugin] fix destination paths in _do_copy_path()

The path used to copy special device nodes and directories in

_do_copy_path() should be the destination path in the archive

(without sysroot prefix), and not the source path in the host

file system that includes this prefix.

Related: #1842

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

---

 sos/plugins/__init__.py | 6 +++---

 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sos/plugins/__init__.py b/sos/plugins/__init__.py

index 4f1b73ce..60fbeaf7 100644

--- a/sos/plugins/__init__.py

+++ b/sos/plugins/__init__.py

@@ -721,8 +721,12 @@ class Plugin(object):

             return

         else:

             if stat.S_ISDIR(st.st_mode) and os.access(srcpath, os.R_OK):

-                self._copy_dir(srcpath)

-                return

+                 # copy empty directory

+                 if not os.listdir(srcpath):

+                     self.archive.add_dir(dest)

+                     return

+                 self._copy_dir(dest)

+                 return

         # handle special nodes (block, char, fifo, socket)

         if not (stat.S_ISREG(st.st_mode) or stat.S_ISDIR(st.st_mode)):

@@ -808,7 +808,7 @@ class Plugin(object):

             ntype = _node_type(st)

             self._log_debug("creating %s node at archive:'%s'"

                             % (ntype, dest))

-            self._copy_node(srcpath, st)

+            self._copy_node(dest, st)

             return

         # if we get here, it's definitely a regular file (not a symlink or dir)

-- 

2.21.0

From 05f3d5bda8f548459fabcd38f2d087d6ecef98a2 Mon Sep 17 00:00:00 2001

From: "Bryn M. Reeves" <bmr@redhat.com>

Date: Fri, 1 Nov 2019 12:25:09 -0400

Subject: [PATCH 03/10] [kernel] remove trailing directory globs in forbidden

 paths

Since the forbidden path test now uses an exact match the trailing

globs ("/some/directory/path/to/exclude/*") used to exclude trace

related directories from collection lead to a failure to properly

blacklist these files:

The glob is expanded, for e.g.:

  "/sys/kernel/debug/tracing/per_cpu/*"

Expands to unclude a 'cpuN' sub-directory for each CPU present on

the machine. These expanded paths are then added to the forbidden

paths list for the plugin:

  /sys/kernel/debug/tracing/per_cpu/cpu0

  /sys/kernel/debug/tracing/per_cpu/cpu1

  ...

When an attempt is made to collect the entire "per_cpu" directory

a check is made for the full "/sys/kernel/debug/tracing/per_cpu"

path against each entry in the forbidden paths list. Since this is

a prefix of the actual paths stored no match is returned and the

collection is permitted.

Remove the trailing globs from these directory paths and prevent

any collection of the directories they reference by the plugin.

Related: #1842

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

---

 sos/plugins/kernel.py | 6 +++---

 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sos/plugins/kernel.py b/sos/plugins/kernel.py

index 88b14689..5c852143 100644

--- a/sos/plugins/kernel.py

+++ b/sos/plugins/kernel.py

@@ -89,9 +89,9 @@ class Kernel(Plugin, RedHatPlugin, DebianPlugin, UbuntuPlugin):

         self.add_forbidden_path([

             '/sys/kernel/debug/tracing/trace_pipe',

             '/sys/kernel/debug/tracing/README',

-            '/sys/kernel/debug/tracing/trace_stat/*',

-            '/sys/kernel/debug/tracing/per_cpu/*',

-            '/sys/kernel/debug/tracing/events/*',

+            '/sys/kernel/debug/tracing/trace_stat',

+            '/sys/kernel/debug/tracing/per_cpu',

+            '/sys/kernel/debug/tracing/events',

             '/sys/kernel/debug/tracing/free_buffer',

             '/sys/kernel/debug/tracing/trace_marker',

             '/sys/kernel/debug/tracing/trace_marker_raw',

-- 

2.21.0

From 801c71b33dcfeaa980baa9f377b721bdd26aa5e8 Mon Sep 17 00:00:00 2001

From: "Bryn M. Reeves" <bmr@redhat.com>

Date: Fri, 1 Nov 2019 16:53:29 +0000

Subject: [PATCH 04/10] [tests] fix test_copy_dir_forbidden_path

Rather than call just Plugin.setup() and Plugin._do_copy_path(),

add an add_copy_spec() call to the mock plugin setup() method,

and invoke copying by calling the Plugin.collect() method.

Related: #1845

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

---

 tests/plugin_tests.py | 3 ++-

 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/plugin_tests.py b/tests/plugin_tests.py

index b8760429..6522fe14 100644

--- a/tests/plugin_tests.py

+++ b/tests/plugin_tests.py

@@ -81,6 +81,7 @@ class ForbiddenMockPlugin(Plugin):

     plugin_name = "forbidden"

     def setup(self):

+        self.add_copy_spec("tests")

         self.add_forbidden_path("tests")

@@ -235,7 +236,7 @@ class PluginTests(unittest.TestCase):

         })

         p.archive = MockArchive()

         p.setup()

-        p._do_copy_path("tests")

+        p.collect()

         self.assertEquals(p.archive.m, {})

-- 

2.21.0

From c4182ebd52af523261d2e7ef75affbb88eaf31fb Mon Sep 17 00:00:00 2001

From: "Bryn M. Reeves" <bmr@redhat.com>

Date: Mon, 4 Nov 2019 10:45:15 +0000

Subject: [PATCH 05/10] [Plugin] use correct source path when copying

 directories

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

---

 sos/plugins/__init__.py | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sos/plugins/__init__.py b/sos/plugins/__init__.py

index 60fbeaf7..240fe9f1 100644

--- a/sos/plugins/__init__.py

+++ b/sos/plugins/__init__.py

@@ -725,7 +725,7 @@ class Plugin(object):

                  if not os.listdir(srcpath):

                      self.archive.add_dir(dest)

                      return

-                 self._copy_dir(dest)

+                 self._copy_dir(srcpath)

                  return

         # handle special nodes (block, char, fifo, socket)

-- 

2.21.0

From 68f4d7cc7adde00171af842b5bc808f41d888a87 Mon Sep 17 00:00:00 2001

From: "Bryn M. Reeves" <bmr@redhat.com>

Date: Mon, 4 Nov 2019 10:48:01 +0000

Subject: [PATCH 06/10] [Plugin] improve _copy_dir() variable naming

Directory entries found in _copy_dir() may be either files or

sub-directories: reflect this in the names of local variables.

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

---

 sos/plugins/__init__.py | 7 ++++---

 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/sos/plugins/__init__.py b/sos/plugins/__init__.py

index 240fe9f1..1a1464c1 100644

--- a/sos/plugins/__init__.py

+++ b/sos/plugins/__init__.py

@@ -738,10 +738,11 @@ class Plugin(object):

     def _copy_dir(self, srcpath):

         try:

-            for afile in os.listdir(srcpath):

+            for name in os.listdir(srcpath):

                 self._log_debug("recursively adding '%s' from '%s'"

-                                % (afile, srcpath))

-                self._do_copy_path(os.path.join(srcpath, afile), dest=None)

+                                % (name, srcpath))

+                path = os.path.join(srcpath, name)

+                self._do_copy_path(path)

         except OSError as e:

             if e.errno == errno.ELOOP:

                 msg = "Too many levels of symbolic links copying"

-- 

2.21.0

From ad3adef07c32aee5bdd438706c6c1d4590ff8297 Mon Sep 17 00:00:00 2001

From: "Bryn M. Reeves" <bmr@redhat.com>

Date: Mon, 4 Nov 2019 14:13:00 +0000

Subject: [PATCH 07/10] [ceph] fix directory blacklist style

Plugins must use 'path/to/exclude' rather than 'path/to/exclude/*'

in order to omit a directory and all its content from the report.

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

---

 sos/plugins/ceph.py | 4 ++--

 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sos/plugins/ceph.py b/sos/plugins/ceph.py

index 6e340c69..43284bc8 100644

--- a/sos/plugins/ceph.py

+++ b/sos/plugins/ceph.py

@@ -103,8 +103,8 @@ class Ceph(Plugin, RedHatPlugin, UbuntuPlugin):

             "/var/lib/ceph/*keyring*",

             "/var/lib/ceph/*/*keyring*",

             "/var/lib/ceph/*/*/*keyring*",

-            "/var/lib/ceph/osd/*",

-            "/var/lib/ceph/mon/*",

+            "/var/lib/ceph/osd",

+            "/var/lib/ceph/mon",

             # Excludes temporary ceph-osd mount location like

             # /var/lib/ceph/tmp/mnt.XXXX from sos collection.

             "/var/lib/ceph/tmp/*mnt*",

-- 

2.21.0

From 4d1576b04d35902ce44d26d6a5b2219e6f9c175a Mon Sep 17 00:00:00 2001

From: "Bryn M. Reeves" <bmr@redhat.com>

Date: Mon, 4 Nov 2019 14:15:55 +0000

Subject: [PATCH 09/10] [openstack_octavia] fix directory blacklist style

Plugins must use 'path/to/exclude' rather than 'path/to/exclude/*'

in order to omit a directory and all its content from the report.

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

---

 sos/plugins/openstack_octavia.py | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sos/plugins/openstack_octavia.py b/sos/plugins/openstack_octavia.py

index b97c83fa..ccdcd4c9 100644

--- a/sos/plugins/openstack_octavia.py

+++ b/sos/plugins/openstack_octavia.py

@@ -30,7 +30,7 @@ class OpenStackOctavia(Plugin):

         ])

         # don't collect certificates

-        self.add_forbidden_path("/etc/octavia/certs/")

+        self.add_forbidden_path("/etc/octavia/certs")

         # logs

         if self.get_option("all_logs"):

-- 

2.21.0

From 1fd194191a56c51052f0c24ddeb3bbf9088ae0ca Mon Sep 17 00:00:00 2001

From: "Bryn M. Reeves" <bmr@redhat.com>

Date: Mon, 4 Nov 2019 14:16:13 +0000

Subject: [PATCH 10/10] [vdsm] fix directory blacklist style

Plugins must use 'path/to/exclude' rather than 'path/to/exclude/*'

in order to omit a directory and all its content from the report.

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

---

 sos/plugins/vdsm.py | 4 ++--

 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sos/plugins/vdsm.py b/sos/plugins/vdsm.py

index b2a1ca58..69672643 100644

--- a/sos/plugins/vdsm.py

+++ b/sos/plugins/vdsm.py

@@ -60,9 +60,9 @@ class Vdsm(Plugin, RedHatPlugin):

     plugin_name = 'vdsm'

     def setup(self):

-        self.add_forbidden_path('/etc/pki/vdsm/keys/*')

+        self.add_forbidden_path('/etc/pki/vdsm/keys')

         self.add_forbidden_path('/etc/pki/vdsm/libvirt-spice/*-key.*')

-        self.add_forbidden_path('/etc/pki/libvirt/private/*')

+        self.add_forbidden_path('/etc/pki/libvirt/private')

         self.add_cmd_output('service vdsmd status')

         self.add_cmd_output('service supervdsmd status')

-- 

2.21.0

From 87dbc4d961d9e98f4e8b7b54010800ff3bdb5a73 Mon Sep 17 00:00:00 2001

From: Jake Hunsaker <jhunsake@redhat.com>

Date: Mon, 11 Nov 2019 12:43:01 -0500

Subject: [PATCH] [Plugin|Policy] Only call lsmod once and standardize kmod

 checks

This commit makes two changes to how sos deals with kernel modules and

their state during a run of sosreport.

First, no longer call `lsmod` for every individual plugin during its

enablement check. Instead, call `lsmod` only once during `Policy`

initialization, and cache the output for later checks.

Second, have `Plugin.is_module_loaded()` check for kmod presence in the

saved policy class attr for kernel_mods, rather than checking through

`/proc/modules`. Have the plugin enablement checks now also use

`is_module_loaded()` to standardize with how `SoSPredicate`s are

checked.

Note that this change results in a significant performance increase for

sos initialization times in a RHEL 7 container.

Resolves: #1854

Signed-off-by: Jake Hunsaker <jhunsake@redhat.com>

---

 sos/plugins/__init__.py  | 13 +++----------

 sos/policies/__init__.py |  8 +++++---

 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/sos/plugins/__init__.py b/sos/plugins/__init__.py

index 1a1464c1..b7a47b6a 100644

--- a/sos/plugins/__init__.py

+++ b/sos/plugins/__init__.py

@@ -1036,11 +1036,8 @@ class Plugin(object):

                                         sizelimit=sizelimit)

     def is_module_loaded(self, module_name):

-        """Return whether specified moudle as module_name is loaded or not"""

-        if len(grep("^" + module_name + " ", "/proc/modules")) == 0:

-            return False

-        else:

-            return True

+        """Return whether specified module as module_name is loaded or not"""

+        return module_name in self.policy.kernel_mods

     # For adding output

     def add_alert(self, alertstring):

@@ -1541,15 +1538,11 @@ class Plugin(object):

         return True

     def _check_plugin_triggers(self, files, packages, commands, services):

-        kernel_mods = self.policy.lsmod()

-

-        def have_kmod(kmod):

-            return kmod in kernel_mods

         return (any(os.path.exists(fname) for fname in files) or

                 any(self.is_installed(pkg) for pkg in packages) or

                 any(is_executable(cmd) for cmd in commands) or

-                any(have_kmod(kmod) for kmod in self.kernel_mods) or

+                any(self.is_module_loaded(mod) for mod in self.kernel_mods) or

                 any(self.is_service(svc) for svc in services))

     def default_enabled(self):

diff --git a/sos/policies/__init__.py b/sos/policies/__init__.py

index a19daf22..f4aa3180 100644

--- a/sos/policies/__init__.py

+++ b/sos/policies/__init__.py

@@ -822,6 +822,7 @@ class LinuxPolicy(Policy):

     def __init__(self, sysroot=None):

         super(LinuxPolicy, self).__init__(sysroot=sysroot)

+        self.init_kernel_modules()

         if self.init == 'systemd':

             self.init_system = SystemdInit()

         else:

@@ -874,11 +875,12 @@ class LinuxPolicy(Policy):

     def sanitize_filename(self, name):

         return re.sub(r"[^-a-z,A-Z.0-9]", "", name)

-    def lsmod(self):

-        """Return a list of kernel module names as strings.

+    def init_kernel_modules(self):

+        """Obtain a list of loaded kernel modules to reference later for plugin

+        enablement and SoSPredicate checks

         """

         lines = shell_out("lsmod", timeout=0).splitlines()

-        return [line.split()[0].strip() for line in lines]

+        self.kernel_mods = [line.split()[0].strip() for line in lines]

     def pre_work(self):

         # this method will be called before the gathering begins

-- 

2.21.0

From 3006f467e6e3908193d28d76bddcc372c4b98875 Mon Sep 17 00:00:00 2001

From: Pavel Moravec <pmoravec@redhat.com>

Date: Tue, 9 Jul 2019 13:35:28 +0200

Subject: [PATCH] [archive] Handle checking container sysroot in

 _make_leading_paths

Previously, in _make_leading_paths(), checking host file paths did not

account for non / sysroots, for situations where sos is run in a

container and the host's / is actually mounted under /host for example.

This would lead to copy errors when trying to copy symlinks.

This method now will use sysroot if one is set, thus avoiding copy

errors.

Resolves: #1705

Signed-off-by: Jake Hunsaker <jhunsake@redhat.com>

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>

---

 sos/archive.py         | 17 +++++++++++++----

 sos/sosreport.py       |  4 ++--

 tests/archive_tests.py |  3 ++-

 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/sos/archive.py b/sos/archive.py

index dcd6908d..7ab36ce4 100644

--- a/sos/archive.py

+++ b/sos/archive.py

@@ -139,12 +139,13 @@ class FileCacheArchive(Archive):

     _archive_root = ""

     _archive_name = ""

-    def __init__(self, name, tmpdir, policy, threads, enc_opts):

+    def __init__(self, name, tmpdir, policy, threads, enc_opts, sysroot):

         self._name = name

         self._tmp_dir = tmpdir

         self._policy = policy

         self._threads = threads

         self.enc_opts = enc_opts

+        self.sysroot = sysroot

         self._archive_root = os.path.join(tmpdir, name)

         with self._path_lock:

             os.makedirs(self._archive_root, 0o700)

@@ -156,6 +157,13 @@ class FileCacheArchive(Archive):

             name = name.lstrip(os.sep)

         return (os.path.join(self._archive_root, name))

+    def join_sysroot(self, path):

+        if path.startswith(self.sysroot):

+            return path

+        if path[0] == os.sep:

+            path = path[1:]

+        return os.path.join(self.sysroot, path)

+

     def _make_leading_paths(self, src, mode=0o700):

         """Create leading path components

@@ -191,7 +199,8 @@ class FileCacheArchive(Archive):

             src_dir = src

         else:

             # Host file path

-            src_dir = src if os.path.isdir(src) else os.path.split(src)[0]

+            src_dir = (src if os.path.isdir(self.join_sysroot(src))

+                       else os.path.split(src)[0])

         # Build a list of path components in root-to-leaf order.

         path = src_dir

@@ -675,9 +684,9 @@ class TarFileArchive(FileCacheArchive):

     method = None

     _with_selinux_context = False

-    def __init__(self, name, tmpdir, policy, threads, enc_opts):

+    def __init__(self, name, tmpdir, policy, threads, enc_opts, sysroot):

         super(TarFileArchive, self).__init__(name, tmpdir, policy, threads,

-                                             enc_opts)

+                                             enc_opts, sysroot)

         self._suffix = "tar"

         self._archive_name = os.path.join(tmpdir, self.name())

diff --git a/sos/sosreport.py b/sos/sosreport.py

index cd61b625..04cb8615 100644

--- a/sos/sosreport.py

+++ b/sos/sosreport.py

@@ -379,12 +379,12 @@ class SoSReport(object):

             auto_archive = self.policy.get_preferred_archive()

             self.archive = auto_archive(archive_name, self.tmpdir,

                                         self.policy, self.opts.threads,

-                                        enc_opts)

+                                        enc_opts, self.sysroot)

         else:

             self.archive = TarFileArchive(archive_name, self.tmpdir,

                                           self.policy, self.opts.threads,

-                                          enc_opts)

+                                          enc_opts, self.sysroot)

         self.archive.set_debug(True if self.opts.debug else False)

diff --git a/tests/archive_tests.py b/tests/archive_tests.py

index e5b329b5..350220b9 100644

--- a/tests/archive_tests.py

+++ b/tests/archive_tests.py

@@ -20,7 +20,7 @@ class TarFileArchiveTest(unittest.TestCase):

     def setUp(self):

         self.tmpdir = tempfile.mkdtemp()

         enc = {'encrypt': False}

-        self.tf = TarFileArchive('test', self.tmpdir, Policy(), 1, enc)

+        self.tf = TarFileArchive('test', self.tmpdir, Policy(), 1, enc, '/')

     def tearDown(self):

         shutil.rmtree(self.tmpdir)

@@ -113,6 +113,7 @@ class TarFileArchiveTest(unittest.TestCase):

     def test_compress(self):

         self.tf.finalize("auto")

+

 if __name__ == "__main__":

     unittest.main()

-- 

2.21.0