From: Lennart Poettering <lennart@poettering.net>
Date: Mon, 18 Mar 2019 12:14:19 +0000 (+0100)
Subject: core: check start limit on condition checks too
X-Git-Tag: v242-rc1~108^2~3
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2de9b9793b91f492141f090dcc89445511e94bd4;p=thirdparty%2Fsystemd.git

core: check start limit on condition checks too

Let's add a safety precaution: if the start condition checks for a unit
are tested too often and fail each time, let's rate limit this too.

This should add extra safety in case people define .path, .timer or
.automount units that trigger a service that as a conditoin that always
fails.
---

diff --git a/src/core/unit.c b/src/core/unit.c
index d4060a7267e..96b520f3d1c 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1729,7 +1729,7 @@ static bool unit_verify_deps(Unit *u) {
  *         -EBADR:      This unit type does not support starting.
  *         -EALREADY:   Unit is already started.
  *         -EAGAIN:     An operation is already in progress. Retry later.
- *         -ECANCELED:  Too many requests for now.
+ *         -ECANCELED:  Start limit hit, too many requests for now
  *         -ECOMM:      Condition failed
  *         -EPROTO:     Assert failed
  *         -EINVAL:     Unit not loaded
@@ -1741,6 +1741,7 @@ static bool unit_verify_deps(Unit *u) {
 int unit_start(Unit *u) {
         UnitActiveState state;
         Unit *following;
+        int r;
 
         assert(u);
 
@@ -1763,8 +1764,25 @@ int unit_start(Unit *u) {
          * still be useful to speed up activation in case there is some hold-off time, but we don't want to
          * recheck the condition in that case. */
         if (state != UNIT_ACTIVATING &&
-            !unit_test_condition(u))
+            !unit_test_condition(u)) {
+
+                /* Let's also check the start limit here. Normally, the start limit is only checked by the
+                 * .start() method of the unit type after it did some additional checks verifying everything
+                 * is in order (so that those other checks can propagate errors properly). However, if a
+                 * condition check doesn't hold we don't get that far but we should still ensure we are not
+                 * called in a tight loop without a rate limit check enforced, hence do the check here. Note
+                 * that ECOMM is generally not a reason for a job to fail, unlike most other errors here,
+                 * hence the chance is big that any triggering unit for us will trigger us again. Note this
+                 * condition check is a bit different from the condition check inside the per-unit .start()
+                 * function, as this one will not change the unit's state in any way (and we shouldn't here,
+                 * after all the condition failed). */
+
+                r = unit_test_start_limit(u);
+                if (r < 0)
+                        return r;
+
                 return log_unit_debug_errno(u, SYNTHETIC_ERRNO(ECOMM), "Starting requested but condition failed. Not starting unit.");
+        }
 
         /* If the asserts failed, fail the entire job */
         if (state != UNIT_ACTIVATING &&