From 5c873d88bceaf3bbaf2d781a812cbc4415c8b26a Mon Sep 17 00:00:00 2001
From: Eric Garver <eric@garver.life>
Date: Wed, 30 Oct 2019 08:24:46 -0400
Subject: [PATCH 115/122] fix: failure to load modules no longer fatal

There are many cases in which module loading may fail:
 - builtin modules, but corrupt/missing modules.builtin database
 - CONFIG_MODULES=n
 - inside unprivileged container

Unfortunately, we have no way to detect these scenarios. The only thing
we can do is attempt to load the module and hope for the best.

Fixes: #430
Fixes: #519
(cherry picked from commit 88e76ddfed6fe348975bfea9002da0e4627c6e25)
(cherry picked from commit 407af3a6d2f5f0ca6e8a354a8cbfdb3228ad04bf)
---
 src/firewall/core/fw.py             | 17 +++++++++--------
 src/firewall/core/fw_transaction.py | 13 ++++++++-----
 src/tests/functions.at              |  1 +
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/src/firewall/core/fw.py b/src/firewall/core/fw.py
index 3e639f83d1f4..2c4325966a19 100644
--- a/src/firewall/core/fw.py
+++ b/src/firewall/core/fw.py
@@ -385,10 +385,9 @@ class Firewall(object):
         #
         # NOTE: must force loading of nf_conntrack to make sure the values are
         # available in /proc
-        module_return = self.handle_modules(["nf_conntrack"], True)
-        if module_return:
-            log.error("Failed to load nf_conntrack module: %s" % module_return[1])
-            sys.exit(1)
+        (status, msg) = self.handle_modules(["nf_conntrack"], True)
+        if status != 0:
+            log.warning("Failed to load nf_conntrack module: %s" % msg)
         if self._automatic_helpers != "system":
             functions.set_nf_conntrack_helper_setting(self._automatic_helpers == "yes")
         self.nf_conntrack_helper_setting = \
@@ -664,6 +663,8 @@ class Firewall(object):
     # handle modules
 
     def handle_modules(self, _modules, enable):
+        num_failed = 0
+        error_msgs = ""
         for i,module in enumerate(_modules):
             if enable:
                 (status, msg) = self.modules_backend.load_module(module)
@@ -673,9 +674,9 @@ class Firewall(object):
                 else:
                     (status, msg) = self.modules_backend.unload_module(module)
             if status != 0:
-                if enable:
-                    return (_modules[:i], msg) # cleanup modules and error msg
-                # else: ignore cleanup
+                num_failed += 1
+                error_msgs += msg
+                continue
 
             if enable:
                 self._module_refcount.setdefault(module, 0)
@@ -685,7 +686,7 @@ class Firewall(object):
                     self._module_refcount[module] -= 1
                     if self._module_refcount[module] == 0:
                         del self._module_refcount[module]
-        return None
+        return (num_failed, error_msgs)
 
     def _select_firewall_backend(self, backend):
         if backend != "nftables":
diff --git a/src/firewall/core/fw_transaction.py b/src/firewall/core/fw_transaction.py
index ad204c1991cf..d5d3e858b6dd 100644
--- a/src/firewall/core/fw_transaction.py
+++ b/src/firewall/core/fw_transaction.py
@@ -113,11 +113,14 @@ class SimpleFirewallTransaction(object):
         if not error:
             module_return = self.fw.handle_modules(modules, enable)
             if module_return:
-                (cleanup_modules, msg) = module_return
-                if cleanup_modules is not None:
-                    error = True
-                    errorMsg = msg
-                    self.fw.handle_modules(cleanup_modules, not enable)
+                # Debug log about issues loading modules, but don't error. The
+                # modules may be builtin or CONFIG_MODULES=n, in which case
+                # modprobe will fail. Or we may be running inside a container
+                # that doesn't have sufficient privileges. Unfortunately there
+                # is no way for us to know.
+                (status, msg) = module_return
+                if status:
+                    log.debug1(msg)
 
         # error case: revert rules
         if error:
diff --git a/src/tests/functions.at b/src/tests/functions.at
index 209f0f5d2ea9..17ca6c9fc052 100644
--- a/src/tests/functions.at
+++ b/src/tests/functions.at
@@ -143,6 +143,7 @@ m4_define([FWD_END_TEST], [
         IF_IPV6_SUPPORTED([], [
             sed -i "/WARNING: ip6tables not usable, disabling IPv6 firewall/d" ./firewalld.log
         ])
+        sed -i "/modprobe: ERROR:/d" ./firewalld.log
         if test x"$1" != x"ignore"; then
             if test -n "$1"; then
                 sed -i $1 ./firewalld.log
-- 
2.23.0