From 40c05a34595ed769ce676206f3c5de874f9a9234 Mon Sep 17 00:00:00 2001
From: Luca Boccassi <bluca@debian.org>
Date: Wed, 19 Oct 2022 23:52:58 +0100
Subject: [PATCH] service: do fine-grained validation of CPUSchedulingPriority=
 at execution time

The precise bounds of the scheduling priority depend on the scheduling policy,
so depending on the order in which the two settings are specified the
validation might pass or fail.
When checking the setting only validate the outer range (valid values in general are 0 to 99),
and let the execution fail later if the priority does not match the
specified policy (1 to 99 for RR/FIFO, 0 for the rest).

Fixes https://github.com/systemd/systemd/issues/20320
---
 src/core/dbus-execute.c            |  8 ++++----
 src/core/load-fragment.c           | 10 ++++------
 test/units/sched_rr_bad.service    |  4 ++--
 test/units/sched_rr_change.service |  2 +-
 4 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index fd73c7bcb7..c5a51bf5cd 100644
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -2718,15 +2718,15 @@ int bus_exec_context_set_transient_property(
                 return 1;
 
         } else if (streq(name, "CPUSchedulingPriority")) {
-                int32_t p, min, max;
+                int32_t p;
 
                 r = sd_bus_message_read(message, "i", &p);
                 if (r < 0)
                         return r;
 
-                min = sched_get_priority_min(c->cpu_sched_policy);
-                max = sched_get_priority_max(c->cpu_sched_policy);
-                if (p < min || p > max)
+                /* On Linux RR/FIFO range from 1 to 99 and OTHER/BATCH may only be 0. Policy might be set
+                 * later so we do not check the precise range, but only the generic outer bounds. */
+                if (p < 0 || p > 99)
                         return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid CPU scheduling priority: %i", p);
 
                 if (!UNIT_WRITE_FLAGS_NOOP(flags)) {
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index b39cb8cdff..1a5895346d 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -1599,7 +1599,7 @@ int config_parse_exec_cpu_sched_prio(const char *unit,
                                      void *userdata) {
 
         ExecContext *c = ASSERT_PTR(data);
-        int i, min, max, r;
+        int i, r;
 
         assert(filename);
         assert(lvalue);
@@ -1611,11 +1611,9 @@ int config_parse_exec_cpu_sched_prio(const char *unit,
                 return 0;
         }
 
-        /* On Linux RR/FIFO range from 1 to 99 and OTHER/BATCH may only be 0 */
-        min = sched_get_priority_min(c->cpu_sched_policy);
-        max = sched_get_priority_max(c->cpu_sched_policy);
-
-        if (i < min || i > max) {
+        /* On Linux RR/FIFO range from 1 to 99 and OTHER/BATCH may only be 0. Policy might be set later so
+         * we do not check the precise range, but only the generic outer bounds. */
+        if (i < 0 || i > 99) {
                 log_syntax(unit, LOG_WARNING, filename, line, 0, "CPU scheduling priority is out of range, ignoring: %s", rvalue);
                 return 0;
         }
diff --git a/test/units/sched_rr_bad.service b/test/units/sched_rr_bad.service
index c112fdf7cc..b51b868c2a 100644
--- a/test/units/sched_rr_bad.service
+++ b/test/units/sched_rr_bad.service
@@ -4,6 +4,6 @@ Description=Bad sched priority for RR
 
 [Service]
 ExecStart=/bin/true
-CPUSchedulingPolicy=rr
-CPUSchedulingPriority=0
+CPUSchedulingPriority=-1
 CPUSchedulingPriority=100
+CPUSchedulingPolicy=rr
diff --git a/test/units/sched_rr_change.service b/test/units/sched_rr_change.service
index dad7e9bbdf..6ae1febc8f 100644
--- a/test/units/sched_rr_change.service
+++ b/test/units/sched_rr_change.service
@@ -4,7 +4,7 @@ Description=Change prio
 
 [Service]
 ExecStart=/bin/true
-CPUSchedulingPolicy=rr
 CPUSchedulingPriority=1
 CPUSchedulingPriority=2
 CPUSchedulingPriority=99
+CPUSchedulingPolicy=rr