From aaae822b37aa3ca39aebb516fdc6bef36d730c25 Mon Sep 17 00:00:00 2001
From: Daan De Meyer <daan.j.demeyer@gmail.com>
Date: Fri, 17 Dec 2021 20:01:31 +0100
Subject: [PATCH] core: Add trigger limit for path units

When conditions fail on a service unit, a path unit can cause
PID 1 to busy loop as it keeps trying to activate the service unit.
To avoid this from happening, add a trigger limit to the path unit,
identical to the trigger limit we have for socket units.

Initially, let's start with a high limit and not make it configurable.
If needed, we can add properties to configure the rate limit similar
to the ones we have for socket units.

Conflict:code context adaptation
Reference:https://github.com/systemd/systemd-stable/commit/aaae822b37aa3ca39aebb516fdc6bef36d730c25
---
 src/core/path.c                        | 10 ++++++++++
 src/core/path.h                        |  3 +++
 test/testsuite-63.units/test63.service |  2 +-
 test/units/testsuite-63.service        | 21 +++++++++++++++++----
 4 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/core/path.c b/src/core/path.c
index 1b0f115dd8..f89e35a001 100644
--- a/src/core/path.c
+++ b/src/core/path.c
@@ -265,6 +265,9 @@ static void path_init(Unit *u) {
         assert(u->load_state == UNIT_STUB);
 
         p->directory_mode = 0755;
+
+        p->trigger_limit.interval = 2 * USEC_PER_SEC;
+        p->trigger_limit.burst = 200;
 }
 
 void path_free_specs(Path *p) {
@@ -494,6 +497,12 @@ static void path_enter_running(Path *p) {
         if (unit_stop_pending(UNIT(p)))
                 return;
 
+        if (!ratelimit_below(&p->trigger_limit)) {
+                log_unit_warning(UNIT(p), "Trigger limit hit, refusing further activation.");
+                path_enter_dead(p, PATH_FAILURE_TRIGGER_LIMIT_HIT);
+                return;
+        }
+
         trigger = UNIT_TRIGGER(UNIT(p));
         if (!trigger) {
                 log_unit_error(UNIT(p), "Unit to trigger vanished.");
@@ -836,6 +845,7 @@ static const char* const path_result_table[_PATH_RESULT_MAX] = {
         [PATH_FAILURE_RESOURCES] = "resources",
         [PATH_FAILURE_START_LIMIT_HIT] = "start-limit-hit",
         [PATH_FAILURE_UNIT_START_LIMIT_HIT] = "unit-start-limit-hit",
+        [PATH_FAILURE_TRIGGER_LIMIT_HIT]    = "trigger-limit-hit",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(path_result, PathResult);
diff --git a/src/core/path.h b/src/core/path.h
index 66ae857dc4..d835c24166 100644
--- a/src/core/path.h
+++ b/src/core/path.h
@@ -46,6 +46,7 @@ typedef enum PathResult {
         PATH_FAILURE_RESOURCES,
         PATH_FAILURE_START_LIMIT_HIT,
         PATH_FAILURE_UNIT_START_LIMIT_HIT,
+        PATH_FAILURE_TRIGGER_LIMIT_HIT,
         _PATH_RESULT_MAX,
         _PATH_RESULT_INVALID = -EINVAL,
 } PathResult;
@@ -61,6 +62,8 @@ struct Path {
         mode_t directory_mode;
 
         PathResult result;
+
+        RateLimit trigger_limit;
 };
 
 void path_free_specs(Path *p);
diff --git a/test/testsuite-63.units/test63.service b/test/testsuite-63.units/test63.service
index c838018..6292434 100644
--- a/test/testsuite-63.units/test63.service
+++ b/test/testsuite-63.units/test63.service
@@ -1,5 +1,5 @@
 [Unit]
-ConditionPathExists=!/tmp/nonexistent
+ConditionPathExists=/tmp/nonexistent
 
 [Service]
 ExecStart=true
diff --git a/test/units/testsuite-63.service b/test/units/testsuite-63.service
index 0a8d143be9..40422127ff 100644
--- a/test/units/testsuite-63.service
+++ b/test/units/testsuite-63.service
@@ -5,13 +5,26 @@ Description=TEST-63-ISSUE-17433
 [Service]
 ExecStartPre=rm -f /failed /testok
 Type=oneshot
+
+# Test that a path unit continuously triggering a service that fails condition checks eventually fails with
+# the trigger-limit-hit error.
 ExecStart=rm -f /tmp/nonexistent
 ExecStart=systemctl start test63.path
 ExecStart=touch /tmp/test63
-# Make sure systemd has sufficient time to hit the start limit for test63.service.
+# Make sure systemd has sufficient time to hit the trigger limit for test63.path.
 ExecStart=sleep 2
-ExecStart=sh -x -c 'test "$(systemctl show test63.service -P ActiveState)" = failed'
-ExecStart=sh -x -c 'test "$(systemctl show test63.service -P Result)" = start-limit-hit'
+ExecStart=sh -x -c 'test "$(systemctl show test63.service -P ActiveState)" = inactive'
+ExecStart=sh -x -c 'test "$(systemctl show test63.service -P Result)" = success'
 ExecStart=sh -x -c 'test "$(systemctl show test63.path -P ActiveState)" = failed'
-ExecStart=sh -x -c 'test "$(systemctl show test63.path -P Result)" = unit-start-limit-hit'
+ExecStart=sh -x -c 'test "$(systemctl show test63.path -P Result)" = trigger-limit-hit'
+
+# Test that starting the service manually doesn't affect the path unit.
+ExecStart=rm -f /tmp/test63
+ExecStart=systemctl reset-failed
+ExecStart=systemctl start test63.path
+ExecStart=systemctl start test63.service
+ExecStart=sh -x -c 'test "$(systemctl show test63.service -P ActiveState)" = inactive'
+ExecStart=sh -x -c 'test "$(systemctl show test63.service -P Result)" = success'
+ExecStart=sh -x -c 'test "$(systemctl show test63.path -P ActiveState)" = active'
+ExecStart=sh -x -c 'test "$(systemctl show test63.path -P Result)" = success'
 ExecStart=sh -x -c 'echo OK >/testok'
-- 
2.33.0

